#9900: Accurate implementation of H3 Request.beginNanoTime()

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
jetty · Nov 23, 2023 · bd7d858 · bd7d858
1 parent 4a91cb7
commit bd7d858
Show file tree

Hide file tree

Showing 10 changed files with 58 additions and 12 deletions.
diff --git a/...-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/parser/MessageParser.java b/...-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/parser/MessageParser.java
@@ -44,22 +44,19 @@ public class MessageParser
     private final BooleanSupplier isLast;
     private BodyParser unknownBodyParser;
     private State state = State.HEADER;
-    protected boolean dataMode;
     private long beginNanoTime;
+    private boolean beginNanoTimeStored;
+    protected boolean dataMode;
 
     public MessageParser(ParserListener listener, QpackDecoder decoder, long streamId, BooleanSupplier isLast)
     {
         this.listener = listener;
         this.decoder = decoder;
+        decoder.setBeginNanoTimeSupplier(this::getBeginNanoTime);
         this.streamId = streamId;
         this.isLast = isLast;
     }
 
-    public long getBeginNanoTime()
-    {
-        return beginNanoTime;
-    }
-
     public void init(UnaryOperator<ParserListener> wrapper)
     {
         ParserListener listener = wrapper.apply(this.listener);
@@ -73,6 +70,21 @@ private void reset()
     {
         headerParser.reset();
         state = State.HEADER;
+        beginNanoTimeStored = false;
+    }
+
+    private void storeBeginNanoTime()
+    {
+        if (!beginNanoTimeStored)
+        {
+            beginNanoTime = NanoTime.now();
+            beginNanoTimeStored = true;
+        }
+    }
+
+    private long getBeginNanoTime()
+    {
+        return beginNanoTime;
     }
 
     public ParserListener getListener()
@@ -108,6 +120,7 @@ public Result parse(ByteBuffer buffer)
                 {
                     case HEADER ->
                     {
+                        storeBeginNanoTime();
                         if (headerParser.parse(buffer))
                         {
                             state = State.BODY;
@@ -124,7 +137,6 @@ public Result parse(ByteBuffer buffer)
                     {
                         BodyParser bodyParser = null;
                         long frameType = headerParser.getFrameType();
-                        beginNanoTime = NanoTime.now(); // TODO #9900 check beginNanoTime's accuracy
                         if (frameType >= 0 && frameType < bodyParsers.length)
                             bodyParser = bodyParsers[(int)frameType];
 

diff --git a/...ty-http3-common/src/test/java/org/eclipse/jetty/http3/internal/DataGenerateParseTest.java b/...ty-http3-common/src/test/java/org/eclipse/jetty/http3/internal/DataGenerateParseTest.java
@@ -23,8 +23,10 @@
 import org.eclipse.jetty.http3.generator.MessageGenerator;
 import org.eclipse.jetty.http3.parser.MessageParser;
 import org.eclipse.jetty.http3.parser.ParserListener;
+import org.eclipse.jetty.http3.qpack.QpackDecoder;
 import org.eclipse.jetty.io.ByteBufferPool;
 import org.eclipse.jetty.util.BufferUtil;
+import org.eclipse.jetty.util.NanoTime;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
@@ -58,14 +60,16 @@ private void testGenerateParse(ByteBuffer byteBuffer)
         new MessageGenerator(bufferPool, null, true).generate(accumulator, 0, input, null);
 
         List<DataFrame> frames = new ArrayList<>();
+        QpackDecoder decoder = new QpackDecoder(instructions -> {});
+        decoder.setBeginNanoTimeSupplier(NanoTime::now);
         MessageParser parser = new MessageParser(new ParserListener()
         {
             @Override
             public void onData(long streamId, DataFrame frame)
             {
                 frames.add(frame);
             }
-        }, null, 13, () -> true);
+        }, decoder, 13, () -> true);
         parser.init(UnaryOperator.identity());
         for (ByteBuffer buffer : accumulator.getByteBuffers())
         {

diff --git a/...http3-common/src/test/java/org/eclipse/jetty/http3/internal/HeadersGenerateParseTest.java b/...http3-common/src/test/java/org/eclipse/jetty/http3/internal/HeadersGenerateParseTest.java
@@ -30,6 +30,7 @@
 import org.eclipse.jetty.http3.qpack.QpackDecoder;
 import org.eclipse.jetty.http3.qpack.QpackEncoder;
 import org.eclipse.jetty.io.ByteBufferPool;
+import org.eclipse.jetty.util.NanoTime;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -54,6 +55,7 @@ public void testGenerateParse()
 
         QpackDecoder decoder = new QpackDecoder(instructions -> {});
         decoder.setMaxHeadersSize(4 * 1024);
+        decoder.setBeginNanoTimeSupplier(NanoTime::now);
         List<HeadersFrame> frames = new ArrayList<>();
         MessageParser parser = new MessageParser(new ParserListener()
         {

diff --git a/...tty-http3/jetty-http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackDecoder.java b/...tty-http3/jetty-http3-qpack/src/main/java/org/eclipse/jetty/http3/qpack/QpackDecoder.java
@@ -21,6 +21,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.LongSupplier;
 
 import org.eclipse.jetty.http.HttpField;
 import org.eclipse.jetty.http.MetaData;
@@ -59,6 +60,7 @@ public class QpackDecoder implements Dumpable
     private int _maxHeadersSize;
     private int _maxBlockedStreams;
     private int _maxTableCapacity;
+    private LongSupplier _beginNanoTimeSupplier;
 
     private static class MetaDataNotification
     {
@@ -96,6 +98,11 @@ public int getMaxHeadersSize()
         return _maxHeadersSize;
     }
 
+    public void setBeginNanoTimeSupplier(LongSupplier beginNanoTimeSupplier)
+    {
+        _beginNanoTimeSupplier = beginNanoTimeSupplier;
+    }
+
     /**
      * @param maxHeadersSize The maximum allowed size of a headers block, expressed as total of all name and value characters, plus 32 per field
      */
@@ -173,7 +180,7 @@ public boolean decode(long streamId, ByteBuffer buffer, Handler handler) throws
         {
             // Parse the buffer into an Encoded Field Section.
             int base = signBit ? requiredInsertCount - deltaBase - 1 : requiredInsertCount + deltaBase;
-            EncodedFieldSection encodedFieldSection = new EncodedFieldSection(streamId, handler, requiredInsertCount, base, buffer);
+            EncodedFieldSection encodedFieldSection = new EncodedFieldSection(streamId, handler, requiredInsertCount, base, buffer, _beginNanoTimeSupplier.getAsLong());
 
             // Decode it straight away if we can, otherwise add it to the list of EncodedFieldSections.
             if (requiredInsertCount <= insertCount)

diff --git a/...-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/metadata/MetaDataBuilder.java b/...-qpack/src/main/java/org/eclipse/jetty/http3/qpack/internal/metadata/MetaDataBuilder.java
@@ -41,6 +41,7 @@ public class MetaDataBuilder
     private QpackException.StreamException _streamException;
     private boolean _request;
     private boolean _response;
+    private long _beginNanoTime = Long.MIN_VALUE;
 
     /**
      * @param maxHeadersSize The maximum size of the headers, expressed as total name and value characters.
@@ -60,6 +61,13 @@ public int getMaxSize()
         return _maxSize;
     }
 
+    public void setBeginNanoTime(long beginNanoTime)
+    {
+        if (beginNanoTime == Long.MIN_VALUE)
+            beginNanoTime++;
+        _beginNanoTime = beginNanoTime;
+    }
+
     /**
      * Get the size.
      *
@@ -247,11 +255,13 @@ public MetaData build() throws QpackException.StreamException
                     if (_path == null)
                         throw new QpackException.StreamException(H3_GENERAL_PROTOCOL_ERROR, "No Path");
                 }
+                long nanoTime = _beginNanoTime == Long.MIN_VALUE ? NanoTime.now() : _beginNanoTime;
+                _beginNanoTime = Long.MIN_VALUE;
                 if (isConnect)
-                    return new MetaData.ConnectRequest(NanoTime.now(), _scheme, _authority, _path, fields, _protocol); // TODO #9900 make beginNanoTime accurate
+                    return new MetaData.ConnectRequest(nanoTime, _scheme, _authority, _path, fields, _protocol);
                 else
                     return new MetaData.Request(
-                        NanoTime.now(), // TODO #9900 make beginNanoTime accurate
+                        nanoTime,
                         _method,
                         _scheme.asString(),
                         _authority,

diff --git a/...pack/src/main/java/org/eclipse/jetty/http3/qpack/internal/parser/EncodedFieldSection.java b/...pack/src/main/java/org/eclipse/jetty/http3/qpack/internal/parser/EncodedFieldSection.java
@@ -44,13 +44,15 @@ public class EncodedFieldSection
     private final int _requiredInsertCount;
     private final int _base;
     private final QpackDecoder.Handler _handler;
+    private final long _beginNanoTime;
 
-    public EncodedFieldSection(long streamId, QpackDecoder.Handler handler, int requiredInsertCount, int base, ByteBuffer content) throws QpackException
+    public EncodedFieldSection(long streamId, QpackDecoder.Handler handler, int requiredInsertCount, int base, ByteBuffer content, long beginNanoTime) throws QpackException
     {
         _streamId = streamId;
         _requiredInsertCount = requiredInsertCount;
         _base = base;
         _handler = handler;
+        _beginNanoTime = beginNanoTime;
 
         try
         {
@@ -104,6 +106,7 @@ public MetaData decode(QpackContext context, int maxHeaderSize) throws QpackExce
             HttpField decodedField = encodedField.decode(context);
             metaDataBuilder.emit(decodedField);
         }
+        metaDataBuilder.setBeginNanoTime(_beginNanoTime);
         return metaDataBuilder.build();
     }
 

diff --git a/...tp3/jetty-http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/BlockedStreamsTest.java b/...tp3/jetty-http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/BlockedStreamsTest.java
@@ -24,6 +24,7 @@
 import org.eclipse.jetty.http3.qpack.internal.instruction.SectionAcknowledgmentInstruction;
 import org.eclipse.jetty.http3.qpack.internal.instruction.SetCapacityInstruction;
 import org.eclipse.jetty.util.BufferUtil;
+import org.eclipse.jetty.util.NanoTime;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -53,6 +54,7 @@ public void before()
         _decoderHandler = new TestDecoderHandler();
         _encoder = new QpackEncoder(_encoderHandler);
         _decoder = new QpackDecoder(_decoderHandler);
+        _decoder.setBeginNanoTimeSupplier(NanoTime::now);
     }
 
     @Test

diff --git a/...http3/jetty-http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EncodeDecodeTest.java b/...http3/jetty-http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EncodeDecodeTest.java
@@ -28,6 +28,7 @@
 import org.eclipse.jetty.http3.qpack.internal.parser.DecoderInstructionParser;
 import org.eclipse.jetty.http3.qpack.internal.parser.EncoderInstructionParser;
 import org.eclipse.jetty.util.BufferUtil;
+import org.eclipse.jetty.util.NanoTime;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
@@ -65,6 +66,7 @@ protected boolean shouldHuffmanEncode(HttpField httpField)
             }
         };
         _decoder = new QpackDecoder(_decoderHandler);
+        _decoder.setBeginNanoTimeSupplier(NanoTime::now);
 
         _encoderInstructionParser = new EncoderInstructionParser(new EncoderParserDebugHandler(_encoder));
         _decoderInstructionParser = new DecoderInstructionParser(new DecoderParserDebugHandler(_decoder));

diff --git a/...tty-http3/jetty-http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EvictionTest.java b/...tty-http3/jetty-http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/EvictionTest.java
@@ -20,6 +20,7 @@
 import org.eclipse.jetty.http.HttpFields;
 import org.eclipse.jetty.http.HttpVersion;
 import org.eclipse.jetty.http.MetaData;
+import org.eclipse.jetty.util.NanoTime;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -38,6 +39,7 @@ public class EvictionTest
     public void before()
     {
         _decoder = new QpackDecoder(_decoderHandler);
+        _decoder.setBeginNanoTimeSupplier(NanoTime::now);
         _decoder.setMaxHeadersSize(1024);
         _decoder.setMaxTableCapacity(4 * 1024);
 

diff --git a/...ty-http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/SectionAcknowledgmentTest.java b/...ty-http3-qpack/src/test/java/org/eclipse/jetty/http3/qpack/SectionAcknowledgmentTest.java
@@ -18,6 +18,7 @@
 import org.eclipse.jetty.http3.qpack.QpackException.SessionException;
 import org.eclipse.jetty.http3.qpack.internal.instruction.SectionAcknowledgmentInstruction;
 import org.eclipse.jetty.util.BufferUtil;
+import org.eclipse.jetty.util.NanoTime;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -48,6 +49,7 @@ public void before()
         _decoderHandler = new TestDecoderHandler();
         _encoder = new QpackEncoder(_encoderHandler);
         _decoder = new QpackDecoder(_decoderHandler);
+        _decoder.setBeginNanoTimeSupplier(NanoTime::now);
     }
 
     @Test