Skip to content

Commit

Permalink
Merge pull request #5295 from eclipse/jetty-10.0.x-5287-CompressionPool
Browse files Browse the repository at this point in the history 
Issue #5287 - rework CompressionPool to use the jetty-util pool
  • Loading branch information
@lachlan-roberts
lachlan-roberts authored Oct 16, 2020
2 parents 792531d + 4690aa5 commit c95fc2a
Show file tree
Hide file tree
Showing 19 changed files with 236 additions and 154 deletions.
15 changes: 6 additions & 9 deletions jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java
View file
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
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
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
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

0 comments on commit c95fc2a

Please sign in to comment.