From 5cde0ca7c8c29966d5e1567081805dac25b0f930 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 5 May 2021 17:03:38 +1000 Subject: [PATCH] QpackEncoder.encode should take buffer to encode into instead of allocating. Signed-off-by: Lachlan Roberts --- .../jetty/http3/qpack/QpackEncoder.java | 29 +++---------------- .../jetty/http3/qpack/EncodeDecodeTest.java | 5 ++-- .../jetty/http3/qpack/EvictionTest.java | 6 +++- 3 files changed, 12 insertions(+), 28 deletions(-) diff --git a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java index d12afe96c0b..6e1f5061fce 100644 --- a/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java +++ b/jetty-http3/http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackEncoder.java @@ -36,8 +36,6 @@ import org.eclipse.jetty.http3.qpack.internal.parser.EncoderInstructionParser; import org.eclipse.jetty.http3.qpack.internal.table.DynamicTable; import org.eclipse.jetty.http3.qpack.internal.table.Entry; import org.eclipse.jetty.http3.qpack.internal.util.NBitIntegerEncoder; -import org.eclipse.jetty.io.ByteBufferPool; -import org.eclipse.jetty.io.NullByteBufferPool; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.component.Dumpable; import org.slf4j.Logger; @@ -83,27 +81,19 @@ public class QpackEncoder implements Dumpable void onInstruction(Instruction instruction) throws QpackException; } - private final ByteBufferPool _bufferPool; private final Handler _handler; private final QpackContext _context; private final int _maxBlockedStreams; private final Map _streamInfoMap = new HashMap<>(); private final EncoderInstructionParser _parser; - private int _knownInsertCount; + private int _knownInsertCount = 0; private int _blockedStreams = 0; public QpackEncoder(Handler handler, int maxBlockedStreams) - { - this(handler, maxBlockedStreams, new NullByteBufferPool()); - } - - public QpackEncoder(Handler handler, int maxBlockedStreams, ByteBufferPool bufferPool) { _handler = handler; - _bufferPool = bufferPool; _context = new QpackContext(); _maxBlockedStreams = maxBlockedStreams; - _knownInsertCount = 0; _parser = new EncoderInstructionParser(new EncoderAdapter()); } @@ -115,7 +105,6 @@ public class QpackEncoder implements Dumpable /** * Set the capacity of the DynamicTable and send a instruction to set the capacity on the remote Decoder. * @param capacity the new capacity. - * @throws QpackException */ public void setCapacity(int capacity) throws QpackException { @@ -150,7 +139,7 @@ public class QpackEncoder implements Dumpable if (field.getValue() == null) field = new HttpField(field.getHeader(), field.getName(), ""); - boolean canCreateEntry = shouldIndex(field) && dynamicTable.canInsert(field); + boolean canCreateEntry = shouldIndex(field) && dynamicTable.canInsert(field); if (!canCreateEntry) return false; @@ -180,7 +169,7 @@ public class QpackEncoder implements Dumpable } } - public ByteBuffer encode(int streamId, MetaData metadata) throws QpackException + public void encode(ByteBuffer buffer, int streamId, MetaData metadata) throws QpackException { // Verify that we can encode without errors. if (metadata.getFields() != null) @@ -234,16 +223,7 @@ public class QpackEncoder implements Dumpable deltaBase = signBit ? requiredInsertCount - base - 1 : base - requiredInsertCount; } - // Calculate the size required. TODO: it may be more efficient to just use a buffer of MAX_HEADER_SIZE? - int spaceRequired = 0; - spaceRequired += 1 + NBitIntegerEncoder.octectsNeeded(8, encodedInsertCount); - spaceRequired += 1 + NBitIntegerEncoder.octectsNeeded(7, deltaBase); - for (EncodableEntry encodableEntry : encodableEntries) - { - spaceRequired += encodableEntry.getRequiredSize(base); - } - - ByteBuffer buffer = _bufferPool.acquire(spaceRequired, false); + // Encode all the entries into the buffer. int pos = BufferUtil.flipToFill(buffer); // Encode the Field Section Prefix into the ByteBuffer. @@ -258,7 +238,6 @@ public class QpackEncoder implements Dumpable } BufferUtil.flipToFlush(buffer, pos); - return buffer; } private EncodableEntry encode(StreamInfo streamInfo, HttpField field) throws QpackException diff --git a/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EncodeDecodeTest.java b/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EncodeDecodeTest.java index 9750a54564f..effdf9d13ab 100644 --- a/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EncodeDecodeTest.java +++ b/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EncodeDecodeTest.java @@ -77,7 +77,8 @@ public class EncodeDecodeTest int streamId = 0; HttpFields httpFields = HttpFields.build().add(":path", "/index.html"); - ByteBuffer buffer = _encoder.encode(streamId, new MetaData(HttpVersion.HTTP_3, httpFields)); + ByteBuffer buffer = BufferUtil.allocate(1024); + _encoder.encode(buffer, streamId, new MetaData(HttpVersion.HTTP_3, httpFields)); assertNull(_encoderHandler.getInstruction()); assertThat(BufferUtil.toHexString(buffer), QpackTestUtil.equalsHex("0000 510b 2f69 6e64 6578 2e68 746d 6c")); assertTrue(_encoderHandler.isEmpty()); @@ -107,7 +108,7 @@ public class EncodeDecodeTest httpFields = HttpFields.build() .add(":authority", "www.example.com") .add(":path", "/sample/path"); - buffer = _encoder.encode(streamId, new MetaData(HttpVersion.HTTP_3, httpFields)); + _encoder.encode(buffer, streamId, new MetaData(HttpVersion.HTTP_3, httpFields)); instruction = _encoderHandler.getInstruction(); assertThat(instruction, instanceOf(IndexedNameEntryInstruction.class)); diff --git a/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EvictionTest.java b/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EvictionTest.java index 14bb5d095e2..be8fce65c75 100644 --- a/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EvictionTest.java +++ b/jetty-http3/http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EvictionTest.java @@ -20,6 +20,7 @@ import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.util.BufferUtil; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -58,12 +59,14 @@ public class EvictionTest public void test() throws Exception { _encoder.setCapacity(1024); + ByteBuffer encodedFields = BufferUtil.allocate(1024); for (int i = 0; i < 10000; i++) { HttpFields httpFields = newRandomFields(5); int streamId = getPositiveInt(10); - ByteBuffer encodedFields = _encoder.encode(streamId, new MetaData(HttpVersion.HTTP_3, httpFields)); + + _encoder.encode(encodedFields, streamId, new MetaData(HttpVersion.HTTP_3, httpFields)); _decoder.decode(streamId, encodedFields); MetaData result = _decoderHandler.getMetaData(); @@ -77,6 +80,7 @@ public class EvictionTest // System.err.println(); assertTrue(result.getFields().isEqualTo(httpFields)); + BufferUtil.clear(encodedFields); } }