From b239fa07c607dde42bec2a80566dd68822058a40 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 17 Sep 2020 15:17:54 +1000 Subject: [PATCH 01/10] Issue #5287 - rework CompressionPool to use the jetty-util pool Signed-off-by: Lachlan Roberts --- .../util/compression/CompressionPool.java | 98 ++++++++----------- 1 file changed, 40 insertions(+), 58 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java index 5552dbaa9b4b..00d8e4de858c 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java @@ -18,19 +18,14 @@ package org.eclipse.jetty.util.compression; -import java.util.Queue; -import java.util.concurrent.ConcurrentLinkedQueue; -import java.util.concurrent.atomic.AtomicInteger; - +import org.eclipse.jetty.util.Pool; import org.eclipse.jetty.util.component.AbstractLifeCycle; public abstract class CompressionPool extends AbstractLifeCycle { public static final int INFINITE_CAPACITY = -1; - private final Queue _pool; - private final AtomicInteger _numObjects = new AtomicInteger(0); - private int _capacity; + private final Pool _pool; /** * Create a Pool of {@link T} instances. @@ -43,8 +38,7 @@ public abstract class CompressionPool extends AbstractLifeCycle */ public CompressionPool(int capacity) { - _capacity = capacity; - _pool = new ConcurrentLinkedQueue<>(); + _pool = new Pool(capacity, 1); } public int getCapacity() @@ -66,73 +60,61 @@ public void setCapacity(int capacity) /** * @return Object taken from the pool if it is not empty or a newly created Object */ - public T acquire() + public Entry acquire() { - T object; - - if (_capacity == 0) - object = newObject(); - else - { - object = _pool.poll(); - if (object == null) - object = newObject(); - else if (_capacity > 0) - _numObjects.decrementAndGet(); - } - - return object; + return new Entry(_pool.acquire(e -> newObject())); } /** - * @param object returns this Object to the pool or calls {@link #end(Object)} if the pool is full. + * @param entry returns this Object to the pool or calls {@link #end(Object)} if the pool is full. */ - public void release(T object) + public void release(Entry entry) + { + entry.release(); + } + + @Override + public void doStop() + { + // TODO: We can't use this because it will not end the entries after it removes them. + // _pool.close(); + } + + public class Entry { - if (object == null) - return; + private T _value; + private Pool.Entry _entry; - if (_capacity == 0 || !isRunning()) + Entry(Pool.Entry entry) { - end(object); + _entry = entry; + _value = (_entry != null) ? _entry.getPooled() : newObject(); } - else if (_capacity < 0) + + public T get() { - reset(object); - _pool.add(object); + return _value; } - else - { - while (true) - { - int d = _numObjects.get(); - if (d >= _capacity) - { - end(object); - break; - } + public void release() + { + // Reset the value for the next usage. + reset(_value); - if (_numObjects.compareAndSet(d, d + 1)) + if (_entry != null) + { + // If release return false, the entry should be removed and the object should be disposed. + if (!_pool.release(_entry)) { - reset(object); - _pool.add(object); - break; + // TODO: what does it mean if this returns false??? + if (_pool.remove(_entry)) + end(_value); } } - } - } - @Override - public void doStop() - { - T t = _pool.poll(); - while (t != null) - { - end(t); - t = _pool.poll(); + _value = null; + _entry = null; } - _numObjects.set(0); } @Override From 21576f2312a0a4e16f79841160ec3b635e4f9c8a Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 17 Sep 2020 16:43:24 +1000 Subject: [PATCH 02/10] Issue #5287 - fix usages of new CompressionPool Signed-off-by: Lachlan Roberts --- .../jetty/http/GZIPContentDecoder.java | 14 +++---- .../server/handler/gzip/GzipFactory.java | 7 +--- .../server/handler/gzip/GzipHandler.java | 8 +--- .../gzip/GzipHttpOutputInterceptor.java | 42 ++++++++++--------- .../util/compression/CompressionPool.java | 37 ++++++++++++---- .../internal/PerMessageDeflateExtension.java | 26 ++++++------ .../server/jmh/DeflaterPoolBenchmark.java | 5 ++- 7 files changed, 76 insertions(+), 63 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java b/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java index ed4f50b99426..79518f821881 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java @@ -42,9 +42,9 @@ public class GZIPContentDecoder implements Destroyable private static final long UINT_MAX = 0xFFFFFFFFL; private final List _inflateds = new ArrayList<>(); - private final InflaterPool _inflaterPool; private final ByteBufferPool _pool; private final int _bufferSize; + private InflaterPool.Entry _inflaterEntry; private Inflater _inflater; private State _state; private int _size; @@ -64,13 +64,12 @@ public GZIPContentDecoder(int bufferSize) public GZIPContentDecoder(ByteBufferPool pool, int bufferSize) { - this(null, pool, bufferSize); + this(new InflaterPool(0, true), pool, bufferSize); } public GZIPContentDecoder(InflaterPool inflaterPool, ByteBufferPool pool, int bufferSize) { - _inflaterPool = inflaterPool; - _inflater = (inflaterPool == null) ? new Inflater(true) : inflaterPool.acquire(); + _inflaterEntry = inflaterPool.acquire(); _bufferSize = bufferSize; _pool = pool; reset(); @@ -416,11 +415,8 @@ private void reset() @Override public void destroy() { - if (_inflaterPool == null) - _inflater.end(); - else - _inflaterPool.release(_inflater); - + _inflaterEntry.release(); + _inflaterEntry = null; _inflater = null; } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipFactory.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipFactory.java index 3c935e3e5363..81bb859e1ca8 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipFactory.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipFactory.java @@ -18,15 +18,12 @@ package org.eclipse.jetty.server.handler.gzip; -import java.util.zip.Deflater; - import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.util.compression.DeflaterPool; public interface GzipFactory { - Deflater getDeflater(Request request, long contentLength); + DeflaterPool.Entry getDeflaterEntry(Request request, long contentLength); boolean isMimeTypeGzipable(String mimetype); - - void recycle(Deflater deflater); } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java index 670085dc71f5..33d219655ea9 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java @@ -418,7 +418,7 @@ public void addIncludedPaths(String... pathspecs) } @Override - public Deflater getDeflater(Request request, long contentLength) + public DeflaterPool.Entry getDeflaterEntry(Request request, long contentLength) { if (contentLength >= 0 && contentLength < _minGzipSize) { @@ -730,12 +730,6 @@ protected boolean isPathGzipable(String requestURI) return _paths.test(requestURI); } - @Override - public void recycle(Deflater deflater) - { - _deflaterPool.release(deflater); - } - /** * Set the excluded filter list of HTTP methods (replacing any previously set) * diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java index 843e90474a62..86d73d1be346 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java @@ -36,6 +36,7 @@ import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IteratingNestedCallback; import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.compression.DeflaterPool; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -63,7 +64,7 @@ private enum GZState private final int _bufferSize; private final boolean _syncFlush; - private Deflater _deflater; + private DeflaterPool.Entry _deflaterEntry; private ByteBuffer _buffer; public GzipHttpOutputInterceptor(GzipFactory factory, HttpChannel channel, HttpOutput.Interceptor next, boolean syncFlush) @@ -122,7 +123,7 @@ public void write(ByteBuffer content, boolean complete, Callback callback) private void addTrailer() { BufferUtil.putIntLittleEndian(_buffer, (int)_crc.getValue()); - BufferUtil.putIntLittleEndian(_buffer, _deflater.getTotalIn()); + BufferUtil.putIntLittleEndian(_buffer, _deflaterEntry.get().getTotalIn()); } private void gzip(ByteBuffer content, boolean complete, final Callback callback) @@ -195,8 +196,8 @@ protected void commit(ByteBuffer content, boolean complete, Callback callback) if (contentLength < 0 && complete) contentLength = content.remaining(); - _deflater = _factory.getDeflater(_channel.getRequest(), contentLength); - if (_deflater == null) + _deflaterEntry = _factory.getDeflaterEntry(_channel.getRequest(), contentLength); + if (_deflaterEntry == null) { LOG.debug("{} exclude no deflater", this); _state.set(GZState.NOT_COMPRESSING); @@ -213,7 +214,7 @@ protected void commit(ByteBuffer content, boolean complete, Callback callback) if (etag != null) fields.put(HttpHeader.ETAG, etagGzip(etag)); - LOG.debug("{} compressing {}", this, _deflater); + LOG.debug("{} compressing {}", this, _deflaterEntry); _state.set(GZState.COMPRESSING); if (BufferUtil.isEmpty(content)) @@ -277,8 +278,8 @@ public GzipBufferCB(ByteBuffer content, boolean complete, Callback callback) @Override protected void onCompleteFailure(Throwable x) { - _factory.recycle(_deflater); - _deflater = null; + _deflaterEntry.release(); + _deflaterEntry = null; super.onCompleteFailure(x); } @@ -286,7 +287,7 @@ protected void onCompleteFailure(Throwable x) protected Action process() throws Exception { // If we have no deflator - if (_deflater == null) + if (_deflaterEntry == null) { // then the trailer has been generated and written below. // we have finished compressing the entire content, so @@ -318,16 +319,17 @@ protected Action process() throws Exception } // If the deflator is not finished, then compress more data - if (!_deflater.finished()) + Deflater deflater = _deflaterEntry.get(); + if (!deflater.finished()) { - if (_deflater.needsInput()) + if (deflater.needsInput()) { // if there is no more content available to compress // then we are either finished all content or just the current write. if (BufferUtil.isEmpty(_content)) { if (_last) - _deflater.finish(); + deflater.finish(); else return Action.SUCCEEDED; } @@ -356,32 +358,32 @@ protected Action process() throws Exception _crc.update(array, off, len); // Ideally we would want to use the ByteBuffer API for Deflaters. However due the the ByteBuffer implementation // of the CRC32.update() it is less efficient for us to use this rather than to convert to array ourselves. - _deflater.setInput(array, off, len); + _deflaterEntry.get().setInput(array, off, len); slice.position(slice.position() + len); if (_last && BufferUtil.isEmpty(_content)) - _deflater.finish(); + deflater.finish(); } } // deflate the content into the available space in the buffer int off = _buffer.arrayOffset() + _buffer.limit(); int len = BufferUtil.space(_buffer); - int produced = _deflater.deflate(_buffer.array(), off, len, _syncFlush ? Deflater.SYNC_FLUSH : Deflater.NO_FLUSH); + int produced = deflater.deflate(_buffer.array(), off, len, _syncFlush ? Deflater.SYNC_FLUSH : Deflater.NO_FLUSH); _buffer.limit(_buffer.limit() + produced); } // If we have finished deflation and there is room for the trailer. - if (_deflater.finished() && BufferUtil.space(_buffer) >= 8) + if (deflater.finished() && BufferUtil.space(_buffer) >= 8) { // add the trailer and recycle the deflator to flag that we will have had completeSuccess when // the write below completes. addTrailer(); - _factory.recycle(_deflater); - _deflater = null; + _deflaterEntry.release(); + _deflaterEntry = null; } // write the compressed buffer. - _interceptor.write(_buffer, _deflater == null, this); + _interceptor.write(_buffer, _deflaterEntry == null, this); return Action.SCHEDULED; } @@ -394,8 +396,8 @@ public String toString() _last, BufferUtil.toDetailString(_copy), BufferUtil.toDetailString(_buffer), - _deflater, - _deflater != null && _deflater.finished() ? "(finished)" : ""); + _deflaterEntry, + _deflaterEntry != null && _deflaterEntry.get().finished() ? "(finished)" : ""); } } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java index 00d8e4de858c..e7bf4ce699c7 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java @@ -23,9 +23,10 @@ public abstract class CompressionPool extends AbstractLifeCycle { - public static final int INFINITE_CAPACITY = -1; + public static final int INFINITE_CAPACITY = Integer.MAX_VALUE; - private final Pool _pool; + private int _capacity; + private Pool _pool; /** * Create a Pool of {@link T} instances. @@ -38,7 +39,7 @@ public abstract class CompressionPool extends AbstractLifeCycle */ public CompressionPool(int capacity) { - _pool = new Pool(capacity, 1); + _capacity = capacity; } public int getCapacity() @@ -48,6 +49,8 @@ public int getCapacity() public void setCapacity(int capacity) { + if (isStarted()) + throw new IllegalStateException("Already Started"); _capacity = capacity; } @@ -62,7 +65,10 @@ public void setCapacity(int capacity) */ public Entry acquire() { - return new Entry(_pool.acquire(e -> newObject())); + if (_pool != null) + return new Entry(_pool.acquire(e -> newObject())); + else + return new Entry(); } /** @@ -74,10 +80,20 @@ public void release(Entry entry) } @Override - public void doStop() + protected void doStart() throws Exception + { + if (_capacity > 0) + _pool = new Pool<>(Pool.StrategyType.RANDOM, _capacity, true); + super.doStart(); + } + + @Override + public void doStop() throws Exception { - // TODO: We can't use this because it will not end the entries after it removes them. - // _pool.close(); + // TODO: Pool.close() will not end the entries after it removes them. + _pool.close(); + _pool = null; + super.doStop(); } public class Entry @@ -85,6 +101,11 @@ public class Entry private T _value; private Pool.Entry _entry; + Entry() + { + this(null); + } + Entry(Pool.Entry entry) { _entry = entry; @@ -125,6 +146,6 @@ public String toString() hashCode(), getState(), _pool.size(), - _capacity < 0 ? "UNLIMITED" : _capacity); + _capacity); } } diff --git a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/PerMessageDeflateExtension.java b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/PerMessageDeflateExtension.java index a5df5908c34d..66193b5f3450 100644 --- a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/PerMessageDeflateExtension.java +++ b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/PerMessageDeflateExtension.java @@ -27,6 +27,8 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.compression.DeflaterPool; +import org.eclipse.jetty.util.compression.InflaterPool; import org.eclipse.jetty.websocket.core.AbstractExtension; import org.eclipse.jetty.websocket.core.ExtensionConfig; import org.eclipse.jetty.websocket.core.Frame; @@ -52,8 +54,8 @@ public class PerMessageDeflateExtension extends AbstractExtension private final TransformingFlusher outgoingFlusher; private final TransformingFlusher incomingFlusher; - private Deflater deflaterImpl; - private Inflater inflaterImpl; + private DeflaterPool.Entry deflaterEntry; + private InflaterPool.Entry inflaterEntry; private boolean incomingCompressed; private ExtensionConfig configRequested; @@ -178,28 +180,28 @@ public static boolean endsWithTail(ByteBuffer buf) public Deflater getDeflater() { - if (deflaterImpl == null) - deflaterImpl = getDeflaterPool().acquire(); - return deflaterImpl; + if (deflaterEntry == null) + deflaterEntry = getDeflaterPool().acquire(); + return deflaterEntry.get(); } public Inflater getInflater() { - if (inflaterImpl == null) - inflaterImpl = getInflaterPool().acquire(); - return inflaterImpl; + if (inflaterEntry == null) + inflaterEntry = getInflaterPool().acquire(); + return inflaterEntry.get(); } public void releaseInflater() { - getInflaterPool().release(inflaterImpl); - inflaterImpl = null; + inflaterEntry.release(); + inflaterEntry = null; } public void releaseDeflater() { - getDeflaterPool().release(deflaterImpl); - deflaterImpl = null; + deflaterEntry.release(); + deflaterEntry = null; } @Override diff --git a/tests/jetty-jmh/src/main/java/org/eclipse/jetty/server/jmh/DeflaterPoolBenchmark.java b/tests/jetty-jmh/src/main/java/org/eclipse/jetty/server/jmh/DeflaterPoolBenchmark.java index 72500ca72a09..0ab725050aab 100644 --- a/tests/jetty-jmh/src/main/java/org/eclipse/jetty/server/jmh/DeflaterPoolBenchmark.java +++ b/tests/jetty-jmh/src/main/java/org/eclipse/jetty/server/jmh/DeflaterPoolBenchmark.java @@ -92,13 +92,14 @@ public static void stopTrial() throws Exception @SuppressWarnings("deprecation") public long testPool() throws Exception { - Deflater deflater = _pool.acquire(); + DeflaterPool.Entry entry = _pool.acquire(); + Deflater deflater = entry.get(); deflater.setInput(COMPRESSION_STRING.getBytes()); deflater.finish(); byte[] output = new byte[COMPRESSION_STRING.length() + 1]; int compressedDataLength = deflater.deflate(output); - _pool.release(deflater); + _pool.release(entry); return compressedDataLength; } From d241c6694be97614f040ec9d0f8c8b34b0247d23 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 18 Sep 2020 11:44:52 +1000 Subject: [PATCH 03/10] Issue #5287 - Pool the entries instead of just the Deflater/Inflaters Signed-off-by: Lachlan Roberts --- .../util/compression/CompressionPool.java | 43 +++++++++++++------ 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java index e7bf4ce699c7..80ace49383c8 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java @@ -18,6 +18,8 @@ package org.eclipse.jetty.util.compression; +import java.io.Closeable; + import org.eclipse.jetty.util.Pool; import org.eclipse.jetty.util.component.AbstractLifeCycle; @@ -26,7 +28,7 @@ public abstract class CompressionPool extends AbstractLifeCycle public static final int INFINITE_CAPACITY = Integer.MAX_VALUE; private int _capacity; - private Pool _pool; + private Pool _pool; /** * Create a Pool of {@link T} instances. @@ -65,10 +67,18 @@ public void setCapacity(int capacity) */ public Entry acquire() { + Entry entry = null; if (_pool != null) - return new Entry(_pool.acquire(e -> newObject())); - else - return new Entry(); + { + Pool.Entry acquiredEntry = _pool.acquire(e -> new Entry(newObject())); + if (acquiredEntry != null) + { + entry = acquiredEntry.getPooled(); + entry.setEntry(acquiredEntry); + } + } + + return (entry == null) ? new Entry(newObject()) : entry; } /** @@ -90,26 +100,25 @@ protected void doStart() throws Exception @Override public void doStop() throws Exception { - // TODO: Pool.close() will not end the entries after it removes them. _pool.close(); _pool = null; super.doStop(); } - public class Entry + public class Entry implements Closeable { private T _value; - private Pool.Entry _entry; + private Pool.Entry _entry; - Entry() + Entry(T value) { - this(null); + _value = value; + _entry = null; } - Entry(Pool.Entry entry) + void setEntry(Pool.Entry entry) { _entry = entry; - _value = (_entry != null) ? _entry.getPooled() : newObject(); } public T get() @@ -127,12 +136,18 @@ public void release() // If release return false, the entry should be removed and the object should be disposed. if (!_pool.release(_entry)) { - // TODO: what does it mean if this returns false??? if (_pool.remove(_entry)) - end(_value); + close(); } + + _entry = null; } + } + @Override + public void close() + { + end(_value); _value = null; _entry = null; } @@ -145,7 +160,7 @@ public String toString() getClass().getSimpleName(), hashCode(), getState(), - _pool.size(), + (_pool == null) ? -1 : _pool.size(), _capacity); } } From 5dc024298672b227c720961440c2b1787535714f Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 23 Sep 2020 12:10:42 +1000 Subject: [PATCH 04/10] Issue #5287 - Changes from review & fix broken tests from NPE Signed-off-by: Lachlan Roberts --- .../jetty/http/GZIPContentDecoder.java | 1 + .../util/compression/CompressionPool.java | 26 +++++++--------- .../jetty/util/compression/DeflaterPool.java | 2 +- .../jetty/util/compression/InflaterPool.java | 2 +- .../internal/PerMessageDeflateExtension.java | 30 +++++++++++-------- 5 files changed, 32 insertions(+), 29 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java b/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java index 79518f821881..882c06337a2c 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java @@ -70,6 +70,7 @@ public GZIPContentDecoder(ByteBufferPool pool, int bufferSize) public GZIPContentDecoder(InflaterPool inflaterPool, ByteBufferPool pool, int bufferSize) { _inflaterEntry = inflaterPool.acquire(); + _inflater = _inflaterEntry.get(); _bufferSize = bufferSize; _pool = pool; reset(); diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java index 80ace49383c8..f0725bf82309 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java @@ -56,7 +56,7 @@ public void setCapacity(int capacity) _capacity = capacity; } - protected abstract T newObject(); + protected abstract T newPooled(); protected abstract void end(T object); @@ -70,15 +70,12 @@ public Entry acquire() Entry entry = null; if (_pool != null) { - Pool.Entry acquiredEntry = _pool.acquire(e -> new Entry(newObject())); + Pool.Entry acquiredEntry = _pool.acquire(e -> new Entry(newPooled(), e)); if (acquiredEntry != null) - { entry = acquiredEntry.getPooled(); - entry.setEntry(acquiredEntry); - } } - return (entry == null) ? new Entry(newObject()) : entry; + return (entry == null) ? new Entry(newPooled()) : entry; } /** @@ -107,17 +104,20 @@ public void doStop() throws Exception public class Entry implements Closeable { - private T _value; - private Pool.Entry _entry; + private final T _value; + private final Pool.Entry _entry; Entry(T value) { - _value = value; - _entry = null; + this(value, null); } - void setEntry(Pool.Entry entry) + Entry(T value, Pool.Entry entry) { + if (entry != null && entry.getPooled() != value) + throw new IllegalArgumentException("value does not match pooled entry"); + + _value = value; _entry = entry; } @@ -139,8 +139,6 @@ public void release() if (_pool.remove(_entry)) close(); } - - _entry = null; } } @@ -148,8 +146,6 @@ public void release() public void close() { end(_value); - _value = null; - _entry = null; } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/DeflaterPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/DeflaterPool.java index 7fbcc0d1bcb5..ebd3eec2f80e 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/DeflaterPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/DeflaterPool.java @@ -44,7 +44,7 @@ public DeflaterPool(int capacity, int compressionLevel, boolean nowrap) } @Override - protected Deflater newObject() + protected Deflater newPooled() { return new Deflater(compressionLevel, nowrap); } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/InflaterPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/InflaterPool.java index c74b8388386e..a41e1f933c92 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/InflaterPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/InflaterPool.java @@ -41,7 +41,7 @@ public InflaterPool(int capacity, boolean nowrap) } @Override - protected Inflater newObject() + protected Inflater newPooled() { return new Inflater(nowrap); } diff --git a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/PerMessageDeflateExtension.java b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/PerMessageDeflateExtension.java index 66193b5f3450..55f5ead57378 100644 --- a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/PerMessageDeflateExtension.java +++ b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/PerMessageDeflateExtension.java @@ -54,8 +54,8 @@ public class PerMessageDeflateExtension extends AbstractExtension private final TransformingFlusher outgoingFlusher; private final TransformingFlusher incomingFlusher; - private DeflaterPool.Entry deflaterEntry; - private InflaterPool.Entry inflaterEntry; + private DeflaterPool.Entry deflaterHolder; + private InflaterPool.Entry inflaterHolder; private boolean incomingCompressed; private ExtensionConfig configRequested; @@ -180,28 +180,34 @@ public static boolean endsWithTail(ByteBuffer buf) public Deflater getDeflater() { - if (deflaterEntry == null) - deflaterEntry = getDeflaterPool().acquire(); - return deflaterEntry.get(); + if (deflaterHolder == null) + deflaterHolder = getDeflaterPool().acquire(); + return deflaterHolder.get(); } public Inflater getInflater() { - if (inflaterEntry == null) - inflaterEntry = getInflaterPool().acquire(); - return inflaterEntry.get(); + if (inflaterHolder == null) + inflaterHolder = getInflaterPool().acquire(); + return inflaterHolder.get(); } public void releaseInflater() { - inflaterEntry.release(); - inflaterEntry = null; + if (inflaterHolder != null) + { + inflaterHolder.release(); + inflaterHolder = null; + } } public void releaseDeflater() { - deflaterEntry.release(); - deflaterEntry = null; + if (deflaterHolder != null) + { + deflaterHolder.release(); + deflaterHolder = null; + } } @Override From dd06008ff4f10c7ed82abebcc1232193294499c3 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 23 Sep 2020 14:31:57 +1000 Subject: [PATCH 05/10] Issue #5287 - remove IllegalArgumentException from CompressionPool Entry Signed-off-by: Lachlan Roberts --- .../org/eclipse/jetty/util/compression/CompressionPool.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java index f0725bf82309..f3959b52770b 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java @@ -114,9 +114,6 @@ public class Entry implements Closeable Entry(T value, Pool.Entry entry) { - if (entry != null && entry.getPooled() != value) - throw new IllegalArgumentException("value does not match pooled entry"); - _value = value; _entry = entry; } From ef816fcc4266201f17036713be3862801e8b570b Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 23 Sep 2020 17:36:49 +1000 Subject: [PATCH 06/10] DeflaterPoolBenchmark should manage lifecycle of the CompressionPool Signed-off-by: Lachlan Roberts --- .../eclipse/jetty/util/compression/CompressionPool.java | 7 +++++-- .../eclipse/jetty/server/jmh/DeflaterPoolBenchmark.java | 5 +++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java index f3959b52770b..ebf14c6301be 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java @@ -97,8 +97,11 @@ protected void doStart() throws Exception @Override public void doStop() throws Exception { - _pool.close(); - _pool = null; + if (_pool != null) + { + _pool.close(); + _pool = null; + } super.doStop(); } diff --git a/tests/jetty-jmh/src/main/java/org/eclipse/jetty/server/jmh/DeflaterPoolBenchmark.java b/tests/jetty-jmh/src/main/java/org/eclipse/jetty/server/jmh/DeflaterPoolBenchmark.java index 0ab725050aab..a0b14daa79be 100644 --- a/tests/jetty-jmh/src/main/java/org/eclipse/jetty/server/jmh/DeflaterPoolBenchmark.java +++ b/tests/jetty-jmh/src/main/java/org/eclipse/jetty/server/jmh/DeflaterPoolBenchmark.java @@ -80,16 +80,17 @@ public void setupTrial() throws Exception } _pool = new DeflaterPool(capacity, Deflater.DEFAULT_COMPRESSION, true); + _pool.start(); } @TearDown(Level.Trial) - public static void stopTrial() throws Exception + public void stopTrial() throws Exception { + _pool.stop(); } @Benchmark @BenchmarkMode({Mode.Throughput}) - @SuppressWarnings("deprecation") public long testPool() throws Exception { DeflaterPool.Entry entry = _pool.acquire(); From 608a895aab4ad9d66ec18ff4bd1c746e81ff372b Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 23 Sep 2020 17:41:34 +1000 Subject: [PATCH 07/10] Issue #5287 - make default CompressionPool capacity 1024 Signed-off-by: Lachlan Roberts --- .../org/eclipse/jetty/server/handler/gzip/GzipHandler.java | 4 ++-- .../org/eclipse/jetty/util/compression/CompressionPool.java | 2 +- .../eclipse/jetty/websocket/core/WebSocketComponents.java | 4 ++-- .../org/eclipse/jetty/server/jmh/DeflaterPoolBenchmark.java | 6 +++++- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java index 33d219655ea9..4953464368c3 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java @@ -923,12 +923,12 @@ public void setInflaterPoolCapacity(int capacity) protected InflaterPool newInflaterPool() { - return new InflaterPool(CompressionPool.INFINITE_CAPACITY, true); + return new InflaterPool(CompressionPool.DEFAULT_CAPACITY, true); } protected DeflaterPool newDeflaterPool() { - return new DeflaterPool(CompressionPool.INFINITE_CAPACITY, Deflater.DEFAULT_COMPRESSION, true); + return new DeflaterPool(CompressionPool.DEFAULT_CAPACITY, Deflater.DEFAULT_COMPRESSION, true); } @Override diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java index ebf14c6301be..34439d108952 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java @@ -25,7 +25,7 @@ public abstract class CompressionPool extends AbstractLifeCycle { - public static final int INFINITE_CAPACITY = Integer.MAX_VALUE; + public static final int DEFAULT_CAPACITY = 1024; private int _capacity; private Pool _pool; diff --git a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketComponents.java b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketComponents.java index 325132b097bd..5b769e2d1185 100644 --- a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketComponents.java +++ b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketComponents.java @@ -42,8 +42,8 @@ public class WebSocketComponents public WebSocketComponents() { this(new WebSocketExtensionRegistry(), new DecoratedObjectFactory(), new MappedByteBufferPool(), - new InflaterPool(CompressionPool.INFINITE_CAPACITY, true), - new DeflaterPool(CompressionPool.INFINITE_CAPACITY, Deflater.DEFAULT_COMPRESSION, true)); + new InflaterPool(CompressionPool.DEFAULT_CAPACITY, true), + new DeflaterPool(CompressionPool.DEFAULT_CAPACITY, Deflater.DEFAULT_COMPRESSION, true)); } public WebSocketComponents(WebSocketExtensionRegistry extensionRegistry, DecoratedObjectFactory objectFactory, diff --git a/tests/jetty-jmh/src/main/java/org/eclipse/jetty/server/jmh/DeflaterPoolBenchmark.java b/tests/jetty-jmh/src/main/java/org/eclipse/jetty/server/jmh/DeflaterPoolBenchmark.java index a0b14daa79be..36fb06cd1ac4 100644 --- a/tests/jetty-jmh/src/main/java/org/eclipse/jetty/server/jmh/DeflaterPoolBenchmark.java +++ b/tests/jetty-jmh/src/main/java/org/eclipse/jetty/server/jmh/DeflaterPoolBenchmark.java @@ -49,7 +49,7 @@ public class DeflaterPoolBenchmark public static final String COMPRESSION_STRING = "hello world"; DeflaterPool _pool; - @Param({"NO_POOL", "DEFLATER_POOL_10", "DEFLATER_POOL_20", "DEFLATER_POOL_50"}) + @Param({"NO_POOL", "DEFLATER_POOL_10", "DEFLATER_POOL_20", "DEFLATER_POOL_50", "DEFLATER_POOL_DEFAULT"}) public static String poolType; @Setup(Level.Trial) @@ -75,6 +75,10 @@ public void setupTrial() throws Exception capacity = 50; break; + case "DEFLATER_POOL_DEFAULT": + capacity = DeflaterPool.DEFAULT_CAPACITY; + break; + default: throw new IllegalStateException("Unknown poolType Parameter"); } From 7cac3d76bbe974cd3aa2f06fa03b7a11ffbb0377 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 25 Sep 2020 23:49:46 +1000 Subject: [PATCH 08/10] Issue #5287 - close deflater on release if non-pooled entry Signed-off-by: Lachlan Roberts --- .../org/eclipse/jetty/util/compression/CompressionPool.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java index 34439d108952..be50b61ea3bb 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java @@ -140,6 +140,10 @@ public void release() close(); } } + else + { + close(); + } } @Override From 0e3cfe8fc22e8083f04449a2bb85246490fb47e0 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 2 Oct 2020 18:35:27 +1000 Subject: [PATCH 09/10] Issue #5287 - share compression pools and size with max num threads Signed-off-by: Lachlan Roberts --- .../server/handler/gzip/GzipHandler.java | 29 +++++++---------- .../jetty/util/compression/DeflaterPool.java | 19 ++++++++++++ .../jetty/util/compression/InflaterPool.java | 19 ++++++++++++ .../websocket/core/WebSocketComponents.java | 20 ++++++------ .../server/WebSocketServerComponents.java | 31 +++++++++++++++---- ...xWebSocketServletContainerInitializer.java | 2 +- .../JavaxWebSocketServerContainer.java | 2 +- .../server/JettyWebSocketServerContainer.java | 2 +- .../server/JettyWebSocketServlet.java | 2 +- ...yWebSocketServletContainerInitializer.java | 2 +- .../tests/MaxOutgoingFramesTest.java | 2 +- .../util/server/WebSocketUpgradeFilter.java | 2 +- .../server/internal/WebSocketMapping.java | 2 +- 13 files changed, 93 insertions(+), 41 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java index 4953464368c3..6d385f1ace98 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java @@ -43,11 +43,11 @@ import org.eclipse.jetty.http.pathmap.PathSpecSet; import org.eclipse.jetty.server.HttpOutput; import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.handler.HandlerWrapper; import org.eclipse.jetty.util.AsciiLowerCaseSet; import org.eclipse.jetty.util.IncludeExclude; import org.eclipse.jetty.util.StringUtil; -import org.eclipse.jetty.util.compression.CompressionPool; import org.eclipse.jetty.util.compression.DeflaterPool; import org.eclipse.jetty.util.compression.InflaterPool; import org.slf4j.Logger; @@ -163,9 +163,8 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory private static final HttpField TE_CHUNKED = new PreEncodedHttpField(HttpHeader.TRANSFER_ENCODING, HttpHeaderValue.CHUNKED.asString()); private static final Pattern COMMA_GZIP = Pattern.compile(".*, *gzip"); - private final InflaterPool _inflaterPool; - private final DeflaterPool _deflaterPool; - + private InflaterPool _inflaterPool; + private DeflaterPool _deflaterPool; private int _minGzipSize = DEFAULT_MIN_GZIP_SIZE; private boolean _syncFlush = false; private int _inflateBufferSize = -1; @@ -202,11 +201,15 @@ else if (type.startsWith("image/") || if (LOG.isDebugEnabled()) LOG.debug("{} mime types {}", this, _mimeTypes); + } - _deflaterPool = newDeflaterPool(); - _inflaterPool = newInflaterPool(); - addBean(_deflaterPool); - addBean(_inflaterPool); + @Override + protected void doStart() throws Exception + { + Server server = getServer(); + _inflaterPool = InflaterPool.ensurePool(server); + _deflaterPool = DeflaterPool.ensurePool(server); + super.doStart(); } /** @@ -921,16 +924,6 @@ public void setInflaterPoolCapacity(int capacity) _inflaterPool.setCapacity(capacity); } - protected InflaterPool newInflaterPool() - { - return new InflaterPool(CompressionPool.DEFAULT_CAPACITY, true); - } - - protected DeflaterPool newDeflaterPool() - { - return new DeflaterPool(CompressionPool.DEFAULT_CAPACITY, Deflater.DEFAULT_COMPRESSION, true); - } - @Override public String toString() { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/DeflaterPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/DeflaterPool.java index ebd3eec2f80e..434de35d938a 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/DeflaterPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/DeflaterPool.java @@ -20,6 +20,9 @@ import java.util.zip.Deflater; +import org.eclipse.jetty.util.component.ContainerLifeCycle; +import org.eclipse.jetty.util.thread.ThreadPool; + public class DeflaterPool extends CompressionPool { private final int compressionLevel; @@ -60,4 +63,20 @@ protected void reset(Deflater deflater) { deflater.reset(); } + + public static DeflaterPool ensurePool(ContainerLifeCycle containerLifeCycle) + { + DeflaterPool pool = containerLifeCycle.getBean(DeflaterPool.class); + if (pool != null) + return pool; + + int capacity = CompressionPool.DEFAULT_CAPACITY; + ThreadPool.SizedThreadPool threadPool = containerLifeCycle.getBean(ThreadPool.SizedThreadPool.class); + if (threadPool != null) + capacity = threadPool.getMaxThreads(); + + pool = new DeflaterPool(capacity, Deflater.DEFAULT_COMPRESSION, true); + containerLifeCycle.addBean(pool); + return pool; + } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/InflaterPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/InflaterPool.java index a41e1f933c92..866900a22d61 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/InflaterPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/InflaterPool.java @@ -20,6 +20,9 @@ import java.util.zip.Inflater; +import org.eclipse.jetty.util.component.ContainerLifeCycle; +import org.eclipse.jetty.util.thread.ThreadPool; + public class InflaterPool extends CompressionPool { private final boolean nowrap; @@ -57,4 +60,20 @@ protected void reset(Inflater inflater) { inflater.reset(); } + + public static InflaterPool ensurePool(ContainerLifeCycle containerLifeCycle) + { + InflaterPool pool = containerLifeCycle.getBean(InflaterPool.class); + if (pool != null) + return pool; + + int capacity = CompressionPool.DEFAULT_CAPACITY; + ThreadPool.SizedThreadPool threadPool = containerLifeCycle.getBean(ThreadPool.SizedThreadPool.class); + if (threadPool != null) + capacity = threadPool.getMaxThreads(); + + pool = new InflaterPool(capacity, true); + containerLifeCycle.addBean(pool); + return pool; + } } diff --git a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketComponents.java b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketComponents.java index 5b769e2d1185..26b3e9cbaa80 100644 --- a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketComponents.java +++ b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketComponents.java @@ -23,6 +23,7 @@ import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.MappedByteBufferPool; import org.eclipse.jetty.util.DecoratedObjectFactory; +import org.eclipse.jetty.util.component.ContainerLifeCycle; import org.eclipse.jetty.util.compression.CompressionPool; import org.eclipse.jetty.util.compression.DeflaterPool; import org.eclipse.jetty.util.compression.InflaterPool; @@ -31,7 +32,7 @@ * A collection of components which are the resources needed for websockets such as * {@link ByteBufferPool}, {@link WebSocketExtensionRegistry}, and {@link DecoratedObjectFactory}. */ -public class WebSocketComponents +public class WebSocketComponents extends ContainerLifeCycle { private final DecoratedObjectFactory objectFactory; private final WebSocketExtensionRegistry extensionRegistry; @@ -41,19 +42,20 @@ public class WebSocketComponents public WebSocketComponents() { - this(new WebSocketExtensionRegistry(), new DecoratedObjectFactory(), new MappedByteBufferPool(), - new InflaterPool(CompressionPool.DEFAULT_CAPACITY, true), - new DeflaterPool(CompressionPool.DEFAULT_CAPACITY, Deflater.DEFAULT_COMPRESSION, true)); + this(null, null, null, null, null); } public WebSocketComponents(WebSocketExtensionRegistry extensionRegistry, DecoratedObjectFactory objectFactory, ByteBufferPool bufferPool, InflaterPool inflaterPool, DeflaterPool deflaterPool) { - this.extensionRegistry = extensionRegistry; - this.objectFactory = objectFactory; - this.bufferPool = bufferPool; - this.deflaterPool = deflaterPool; - this.inflaterPool = inflaterPool; + this.extensionRegistry = (extensionRegistry == null) ? new WebSocketExtensionRegistry() : extensionRegistry; + this.objectFactory = (objectFactory == null) ? new DecoratedObjectFactory() : objectFactory; + this.bufferPool = (bufferPool == null) ? new MappedByteBufferPool() : bufferPool; + this.inflaterPool = (inflaterPool == null) ? new InflaterPool(CompressionPool.DEFAULT_CAPACITY, true) : inflaterPool; + this.deflaterPool = (deflaterPool == null) ? new DeflaterPool(CompressionPool.DEFAULT_CAPACITY, Deflater.DEFAULT_COMPRESSION, true) : deflaterPool; + + addBean(inflaterPool); + addBean(deflaterPool); } public ByteBufferPool getBufferPool() diff --git a/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java b/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java index 2fefc439d092..1bdc76edf061 100644 --- a/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java +++ b/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java @@ -21,7 +21,10 @@ import javax.servlet.ServletContext; import org.eclipse.jetty.io.ByteBufferPool; +import org.eclipse.jetty.server.Server; import org.eclipse.jetty.util.DecoratedObjectFactory; +import org.eclipse.jetty.util.compression.DeflaterPool; +import org.eclipse.jetty.util.compression.InflaterPool; import org.eclipse.jetty.websocket.core.WebSocketComponents; import org.eclipse.jetty.websocket.core.WebSocketExtensionRegistry; @@ -29,23 +32,39 @@ * A collection of components which are the resources needed for websockets such as * {@link ByteBufferPool}, {@link WebSocketExtensionRegistry}, and {@link DecoratedObjectFactory}. * - * These components should be accessed through {@link WebSocketServerComponents#ensureWebSocketComponents} so that + * These components should be accessed through {@link WebSocketServerComponents#getWebSocketComponents} so that * the instance can be shared by being stored as a bean on the ContextHandler. */ public class WebSocketServerComponents extends WebSocketComponents { public static final String WEBSOCKET_COMPONENTS_ATTRIBUTE = WebSocketComponents.class.getName(); - public static WebSocketComponents ensureWebSocketComponents(ServletContext servletContext) + WebSocketServerComponents(InflaterPool inflaterPool, DeflaterPool deflaterPool) { - // Ensure a mapping exists - WebSocketComponents components = (WebSocketComponents)servletContext.getAttribute(WEBSOCKET_COMPONENTS_ATTRIBUTE); + super(null, null, null, inflaterPool, deflaterPool); + } + + public static WebSocketComponents ensureWebSocketComponents(Server server, ServletContext servletContext) + { + WebSocketComponents components = server.getBean(WebSocketComponents.class); if (components == null) { - components = new WebSocketServerComponents(); - servletContext.setAttribute(WEBSOCKET_COMPONENTS_ATTRIBUTE, components); + InflaterPool inflaterPool = InflaterPool.ensurePool(server); + DeflaterPool deflaterPool = DeflaterPool.ensurePool(server); + components = new WebSocketServerComponents(inflaterPool, deflaterPool); + server.addBean(components); } + servletContext.setAttribute(WEBSOCKET_COMPONENTS_ATTRIBUTE, components); + return components; + } + + public static WebSocketComponents getWebSocketComponents(ServletContext servletContext) + { + WebSocketComponents components = (WebSocketComponents)servletContext.getAttribute(WEBSOCKET_COMPONENTS_ATTRIBUTE); + if (components == null) + throw new IllegalStateException("WebSocketComponents has not been created"); + return components; } } diff --git a/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/config/JavaxWebSocketServletContainerInitializer.java b/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/config/JavaxWebSocketServletContainerInitializer.java index af4cca34fe31..753a454e3b6a 100644 --- a/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/config/JavaxWebSocketServletContainerInitializer.java +++ b/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/config/JavaxWebSocketServletContainerInitializer.java @@ -148,7 +148,7 @@ private static ServerContainer initialize(ServletContextHandler context) JavaxWebSocketServerContainer serverContainer = JavaxWebSocketServerContainer.getContainer(context.getServletContext()); if (serverContainer == null) { - WebSocketComponents components = WebSocketServerComponents.ensureWebSocketComponents(context.getServletContext()); + WebSocketComponents components = WebSocketServerComponents.ensureWebSocketComponents(context.getServer(), context.getServletContext()); FilterHolder filterHolder = WebSocketUpgradeFilter.ensureFilter(context.getServletContext()); WebSocketMapping mapping = WebSocketMapping.ensureMapping(context.getServletContext(), WebSocketMapping.DEFAULT_KEY); serverContainer = JavaxWebSocketServerContainer.ensureContainer(context.getServletContext()); diff --git a/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java b/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java index f3561078a6ec..f9218c819d95 100644 --- a/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java +++ b/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java @@ -99,7 +99,7 @@ public static JavaxWebSocketServerContainer ensureContainer(ServletContext servl // Create the Jetty ServerContainer implementation container = new JavaxWebSocketServerContainer( WebSocketMapping.ensureMapping(servletContext, WebSocketMapping.DEFAULT_KEY), - WebSocketServerComponents.ensureWebSocketComponents(servletContext), + WebSocketServerComponents.getWebSocketComponents(servletContext), coreClientSupplier); contextHandler.addManaged(container); contextHandler.addEventListener(container); diff --git a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java index 52c2d6b2f68c..759e773bbd19 100644 --- a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java +++ b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java @@ -78,7 +78,7 @@ public static JettyWebSocketServerContainer ensureContainer(ServletContext servl container = new JettyWebSocketServerContainer( contextHandler, WebSocketMapping.ensureMapping(servletContext, WebSocketMapping.DEFAULT_KEY), - WebSocketServerComponents.ensureWebSocketComponents(servletContext), executor); + WebSocketServerComponents.getWebSocketComponents(servletContext), executor); servletContext.setAttribute(JETTY_WEBSOCKET_CONTAINER_ATTRIBUTE, container); contextHandler.addManaged(container); contextHandler.addEventListener(container); diff --git a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServlet.java b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServlet.java index 0cbdd6b12127..eb5f4016c5ec 100644 --- a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServlet.java +++ b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServlet.java @@ -132,7 +132,7 @@ public void init() throws ServletException { ServletContext servletContext = getServletContext(); - components = WebSocketServerComponents.ensureWebSocketComponents(servletContext); + components = WebSocketServerComponents.getWebSocketComponents(servletContext); mapping = new WebSocketMapping(components); String max = getInitParameter("idleTimeout"); diff --git a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/config/JettyWebSocketServletContainerInitializer.java b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/config/JettyWebSocketServletContainerInitializer.java index 4740fbc75f88..6b829fd461fa 100644 --- a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/config/JettyWebSocketServletContainerInitializer.java +++ b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/config/JettyWebSocketServletContainerInitializer.java @@ -88,7 +88,7 @@ public static void configure(ServletContextHandler context, Configurator configu */ private static JettyWebSocketServerContainer initialize(ServletContextHandler context) { - WebSocketComponents components = WebSocketServerComponents.ensureWebSocketComponents(context.getServletContext()); + WebSocketComponents components = WebSocketServerComponents.ensureWebSocketComponents(context.getServer(), context.getServletContext()); WebSocketMapping mapping = WebSocketMapping.ensureMapping(context.getServletContext(), WebSocketMapping.DEFAULT_KEY); JettyWebSocketServerContainer container = JettyWebSocketServerContainer.ensureContainer(context.getServletContext()); diff --git a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/MaxOutgoingFramesTest.java b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/MaxOutgoingFramesTest.java index 55cf722ca153..a9bac43cdcd5 100644 --- a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/MaxOutgoingFramesTest.java +++ b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/MaxOutgoingFramesTest.java @@ -73,7 +73,7 @@ public void start() throws Exception JettyWebSocketServletContainerInitializer.configure(contextHandler, (context, container) -> { container.addMapping("/", (req, resp) -> serverSocket); - WebSocketComponents components = WebSocketServerComponents.ensureWebSocketComponents(context); + WebSocketComponents components = WebSocketServerComponents.getWebSocketComponents(context); components.getExtensionRegistry().register(BlockingOutgoingExtension.class.getName(), BlockingOutgoingExtension.class); }); diff --git a/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/WebSocketUpgradeFilter.java b/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/WebSocketUpgradeFilter.java index d756b0bcc182..d3cf9e1391b6 100644 --- a/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/WebSocketUpgradeFilter.java +++ b/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/WebSocketUpgradeFilter.java @@ -177,7 +177,7 @@ public void init(FilterConfig config) throws ServletException if (mappingKey != null) mapping = WebSocketMapping.ensureMapping(context, mappingKey); else - mapping = new WebSocketMapping(WebSocketServerComponents.ensureWebSocketComponents(context)); + mapping = new WebSocketMapping(WebSocketServerComponents.getWebSocketComponents(context)); String max = config.getInitParameter("idleTimeout"); if (max == null) diff --git a/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/internal/WebSocketMapping.java b/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/internal/WebSocketMapping.java index 49283dbf8e1d..5bef84d466f7 100644 --- a/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/internal/WebSocketMapping.java +++ b/jetty-websocket/websocket-util-server/src/main/java/org/eclipse/jetty/websocket/util/server/internal/WebSocketMapping.java @@ -89,7 +89,7 @@ public static WebSocketMapping ensureMapping(ServletContext servletContext, Stri if (mapping == null) { - mapping = new WebSocketMapping(WebSocketServerComponents.ensureWebSocketComponents(servletContext)); + mapping = new WebSocketMapping(WebSocketServerComponents.getWebSocketComponents(servletContext)); servletContext.setAttribute(mappingKey, mapping); } From 4690aa51ba498454b0e27529c252f2b7cc017b3a Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 14 Oct 2020 17:47:50 +1100 Subject: [PATCH 10/10] allow override of shared CompressionPools in WebSocketServerComponents Signed-off-by: Lachlan Roberts --- .../eclipse/jetty/util/compression/DeflaterPool.java | 10 +++++----- .../eclipse/jetty/util/compression/InflaterPool.java | 10 +++++----- .../core/server/WebSocketServerComponents.java | 12 ++++++++++-- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/DeflaterPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/DeflaterPool.java index 434de35d938a..b65a69d3e24a 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/DeflaterPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/DeflaterPool.java @@ -20,7 +20,7 @@ import java.util.zip.Deflater; -import org.eclipse.jetty.util.component.ContainerLifeCycle; +import org.eclipse.jetty.util.component.Container; import org.eclipse.jetty.util.thread.ThreadPool; public class DeflaterPool extends CompressionPool @@ -64,19 +64,19 @@ protected void reset(Deflater deflater) deflater.reset(); } - public static DeflaterPool ensurePool(ContainerLifeCycle containerLifeCycle) + public static DeflaterPool ensurePool(Container container) { - DeflaterPool pool = containerLifeCycle.getBean(DeflaterPool.class); + DeflaterPool pool = container.getBean(DeflaterPool.class); if (pool != null) return pool; int capacity = CompressionPool.DEFAULT_CAPACITY; - ThreadPool.SizedThreadPool threadPool = containerLifeCycle.getBean(ThreadPool.SizedThreadPool.class); + ThreadPool.SizedThreadPool threadPool = container.getBean(ThreadPool.SizedThreadPool.class); if (threadPool != null) capacity = threadPool.getMaxThreads(); pool = new DeflaterPool(capacity, Deflater.DEFAULT_COMPRESSION, true); - containerLifeCycle.addBean(pool); + container.addBean(pool); return pool; } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/InflaterPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/InflaterPool.java index 866900a22d61..25b22a9056ba 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/InflaterPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/InflaterPool.java @@ -20,7 +20,7 @@ import java.util.zip.Inflater; -import org.eclipse.jetty.util.component.ContainerLifeCycle; +import org.eclipse.jetty.util.component.Container; import org.eclipse.jetty.util.thread.ThreadPool; public class InflaterPool extends CompressionPool @@ -61,19 +61,19 @@ protected void reset(Inflater inflater) inflater.reset(); } - public static InflaterPool ensurePool(ContainerLifeCycle containerLifeCycle) + public static InflaterPool ensurePool(Container container) { - InflaterPool pool = containerLifeCycle.getBean(InflaterPool.class); + InflaterPool pool = container.getBean(InflaterPool.class); if (pool != null) return pool; int capacity = CompressionPool.DEFAULT_CAPACITY; - ThreadPool.SizedThreadPool threadPool = containerLifeCycle.getBean(ThreadPool.SizedThreadPool.class); + ThreadPool.SizedThreadPool threadPool = container.getBean(ThreadPool.SizedThreadPool.class); if (threadPool != null) capacity = threadPool.getMaxThreads(); pool = new InflaterPool(capacity, true); - containerLifeCycle.addBean(pool); + container.addBean(pool); return pool; } } diff --git a/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java b/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java index 1bdc76edf061..7529cef2905e 100644 --- a/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java +++ b/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java @@ -38,6 +38,8 @@ public class WebSocketServerComponents extends WebSocketComponents { public static final String WEBSOCKET_COMPONENTS_ATTRIBUTE = WebSocketComponents.class.getName(); + public static final String WEBSOCKET_INFLATER_POOL_ATTRIBUTE = "jetty.websocket.inflater"; + public static final String WEBSOCKET_DEFLATER_POOL_ATTRIBUTE = "jetty.websocket.deflater"; WebSocketServerComponents(InflaterPool inflaterPool, DeflaterPool deflaterPool) { @@ -49,8 +51,14 @@ public static WebSocketComponents ensureWebSocketComponents(Server server, Servl WebSocketComponents components = server.getBean(WebSocketComponents.class); if (components == null) { - InflaterPool inflaterPool = InflaterPool.ensurePool(server); - DeflaterPool deflaterPool = DeflaterPool.ensurePool(server); + InflaterPool inflaterPool = (InflaterPool)servletContext.getAttribute(WEBSOCKET_INFLATER_POOL_ATTRIBUTE); + if (inflaterPool == null) + inflaterPool = InflaterPool.ensurePool(server); + + DeflaterPool deflaterPool = (DeflaterPool)servletContext.getAttribute(WEBSOCKET_DEFLATER_POOL_ATTRIBUTE); + if (deflaterPool == null) + deflaterPool = DeflaterPool.ensurePool(server); + components = new WebSocketServerComponents(inflaterPool, deflaterPool); server.addBean(components); }