Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #5287 - rework CompressionPool to use the jetty-util pool #5295

Merged
merged 10 commits into from
Oct 16, 2020
Merged

Issue #5287 - rework CompressionPool to use the jetty-util pool #5295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b239fa0
Issue #5287 - rework CompressionPool to use the jetty-util pool
lachlan-roberts Sep 17, 2020
21576f2
Issue #5287 - fix usages of new CompressionPool
lachlan-roberts Sep 17, 2020
d241c66
Issue #5287 - Pool the entries instead of just the Deflater/Inflaters
lachlan-roberts Sep 18, 2020
5dc0242
Issue #5287 - Changes from review & fix broken tests from NPE
lachlan-roberts Sep 23, 2020
dd06008
Issue #5287 - remove IllegalArgumentException from CompressionPool Entry
lachlan-roberts Sep 23, 2020
ef816fc
DeflaterPoolBenchmark should manage lifecycle of the CompressionPool
lachlan-roberts Sep 23, 2020
608a895
Issue #5287 - make default CompressionPool capacity 1024
lachlan-roberts Sep 23, 2020
7cac3d7
Issue #5287 - close deflater on release if non-pooled entry
lachlan-roberts Sep 25, 2020
0e3cfe8
Issue #5287 - share compression pools and size with max num threads
lachlan-roberts Oct 2, 2020
4690aa5
allow override of shared CompressionPools in WebSocketServerComponents
lachlan-roberts Oct 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ public class GZIPContentDecoder implements Destroyable
private static final long UINT_MAX = 0xFFFFFFFFL;

private final List<ByteBuffer> _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;
Expand All @@ -64,13 +64,13 @@ 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();
_inflater = _inflaterEntry.get();
_bufferSize = bufferSize;
_pool = pool;
reset();
Expand Down Expand Up @@ -416,11 +416,8 @@ private void reset()
@Override
public void destroy()
{
if (_inflaterPool == null)
_inflater.end();
else
_inflaterPool.release(_inflater);

_inflaterEntry.release();
_inflaterEntry = null;
_inflater = null;
}

Expand Down
7 changes: 2 additions & 5 deletions jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
37 changes: 12 additions & 25 deletions jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}

/**
Expand Down Expand Up @@ -418,7 +421,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)
{
Expand Down Expand Up @@ -730,12 +733,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)
*
Expand Down Expand Up @@ -927,16 +924,6 @@ public void setInflaterPoolCapacity(int capacity)
_inflaterPool.setCapacity(capacity);
}

protected InflaterPool newInflaterPool()
{
return new InflaterPool(CompressionPool.INFINITE_CAPACITY, true);
}

protected DeflaterPool newDeflaterPool()
{
return new DeflaterPool(CompressionPool.INFINITE_CAPACITY, Deflater.DEFAULT_COMPRESSION, true);
}

@Override
public String toString()
{
Expand Down
42 changes: 22 additions & 20 deletions ...server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand All @@ -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))
Expand Down Expand Up @@ -277,16 +278,16 @@ 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);
}

@Override
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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand All @@ -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)" : "");
}
}
}
Loading