HTTP/2 Hpack Encoder Cleanup

Motivation:
The HTTP/2 HPACK Encoder class has some code which is only used for test purposes. This code can be removed to reduce complexity and member variable count.

Modifications:
- Remove test code and update unit tests
- Other minor cleanup

Result:
Test code is removed from operational code.

codec-http2/src/main/java/io/netty/handler/codec/http2/internal/hpack/Encoder.java

@@ -46,11 +46,6 @@ import static java.lang.Math.max;
 import static java.lang.Math.min;
 
 public final class Encoder {
-    // for testing
-    private final boolean useIndexing;
-    private final boolean forceHuffmanOn;
-    private final boolean forceHuffmanOff;
-
     // a linked hash map of header fields
     private final HeaderEntry[] headerFields;
     private final HeaderEntry head = new HeaderEntry(-1, AsciiString.EMPTY_STRING,
@@ -71,25 +66,9 @@ public final class Encoder {
      * Creates a new encoder.
      */
     public Encoder(int maxHeaderTableSize, int arraySizeHint) {
-        this(maxHeaderTableSize, true, false, false, arraySizeHint);
-    }
-
-    /**
-     * Constructor for testing only.
-     */
-    Encoder(
-            int maxHeaderTableSize,
-            boolean useIndexing,
-            boolean forceHuffmanOn,
-            boolean forceHuffmanOff,
-            int arraySizeHint
-    ) {
         if (maxHeaderTableSize < 0) {
             throw new IllegalArgumentException("Illegal Capacity: " + maxHeaderTableSize);
         }
-        this.useIndexing = useIndexing;
-        this.forceHuffmanOn = forceHuffmanOn;
-        this.forceHuffmanOff = forceHuffmanOff;
         capacity = maxHeaderTableSize;
         // Enforce a bound of [2, 128] because hashMask is a byte. The max possible value of hashMask is one less
         // than the length of this array, and we want the mask to be > 0.
@@ -144,19 +123,12 @@ public final class Encoder {
                 // Section 6.1. Indexed Header Field Representation
                 encodeInteger(out, 0x80, 7, staticTableIndex);
             } else {
-                int nameIndex = getNameIndex(name);
-                if (useIndexing) {
                 ensureCapacity(headerSize);
-                }
+                encodeLiteral(out, name, value, INCREMENTAL, getNameIndex(name));
-                HpackUtil.IndexType indexType =
-                        useIndexing ? INCREMENTAL : NONE;
-                encodeLiteral(out, name, value, indexType, nameIndex);
-                if (useIndexing) {
                 add(name, value);
             }
         }
     }
-    }
 
     /**
      * Set the maximum table size.
@@ -184,24 +156,17 @@ public final class Encoder {
      * Encode integer according to Section 5.1.
      */
     private static void encodeInteger(ByteBuf out, int mask, int n, int i) {
-        if (n < 0 || n > 8) {
+        assert n >= 0 && n <= 8 : "N: " + n;
-            throw new IllegalArgumentException("N: " + n);
-        }
         int nbits = 0xFF >>> (8 - n);
         if (i < nbits) {
             out.writeByte(mask | i);
         } else {
             out.writeByte(mask | nbits);
             int length = i - nbits;
-            for (;;) {
+            for (; (length & ~0x7F) != 0; length >>>= 7) {
-                if ((length & ~0x7F) == 0) {
-                    out.writeByte(length);
-                    return;
-                } else {
                 out.writeByte((length & 0x7F) | 0x80);
-                    length >>>= 7;
-                }
             }
+            out.writeByte(length);
         }
     }
 
@@ -210,7 +175,7 @@ public final class Encoder {
      */
     private void encodeStringLiteral(ByteBuf out, CharSequence string) {
         int huffmanLength = huffmanEncoder.getEncodedLength(string);
-        if ((huffmanLength < string.length() && !forceHuffmanOff) || forceHuffmanOn) {
+        if (huffmanLength < string.length()) {
             encodeInteger(out, 0x80, 7, huffmanLength);
             huffmanEncoder.encode(out, string);
         } else {
@@ -232,26 +197,21 @@ public final class Encoder {
      */
     private void encodeLiteral(ByteBuf out, CharSequence name, CharSequence value, HpackUtil.IndexType indexType,
                                int nameIndex) {
-        int mask;
+        boolean nameIndexValid = nameIndex != -1;
-        int prefixBits;
         switch (indexType) {
             case INCREMENTAL:
-                mask = 0x40;
+                encodeInteger(out, 0x40, 6, nameIndexValid ? nameIndex : 0);
-                prefixBits = 6;
                 break;
             case NONE:
-                mask = 0x00;
+                encodeInteger(out, 0x00, 4, nameIndexValid ? nameIndex : 0);
-                prefixBits = 4;
                 break;
             case NEVER:
-                mask = 0x10;
+                encodeInteger(out, 0x10, 4, nameIndexValid ? nameIndex : 0);
-                prefixBits = 4;
                 break;
             default:
-                throw new IllegalStateException("should not reach here");
+                throw new Error("should not reach here");
         }
-        encodeInteger(out, mask, prefixBits, nameIndex == -1 ? 0 : nameIndex);
+        if (!nameIndexValid) {
-        if (nameIndex == -1) {
             encodeStringLiteral(out, name);
         }
         encodeStringLiteral(out, value);

codec-http2/src/test/java/io/netty/handler/codec/http2/internal/hpack/TestCase.java

@@ -61,10 +61,7 @@ final class TestCase {
             .create();
 
     int maxHeaderTableSize = -1;
-    boolean useIndexing = true;
     boolean sensitiveHeaders;
-    boolean forceHuffmanOn;
-    boolean forceHuffmanOff;
 
     List<HeaderBlock> headerBlocks;
 
@@ -161,7 +158,7 @@ final class TestCase {
             maxHeaderTableSize = Integer.MAX_VALUE;
         }
 
-        return new Encoder(maxHeaderTableSize, useIndexing, forceHuffmanOn, forceHuffmanOff, 16);
+        return new Encoder(maxHeaderTableSize);
     }
 
     private Decoder createDecoder() {

codec-http2/src/test/resources/io/netty/handler/codec/http2/internal/hpack/testdata/testDuplicateHeaders.json

@@ -1,5 +1,4 @@
 {
-  "force_huffman_on": true,
   "header_blocks":
   [
     {
@@ -9,7 +8,7 @@
       ],
       "encoded": [
         "4487 6107 a4b5 8d33 ff40 86f2 b12d 424f",
-        "4f83 ee3a 3f"
+        "4f03 7661 6c"
       ],
       "dynamic_table": [
         { "x-custom": "val" },
@@ -40,7 +39,7 @@
         { "x-custom": "val" }
       ],
       "encoded": [
-        "bfbe 4082 94e7 838c 767f bf"
+        "bfbe 4082 94e7 0362 6172 bf"
       ],
       "dynamic_table": [
         { "foo": "bar" },

codec-http2/src/test/resources/io/netty/handler/codec/http2/internal/hpack/testdata/testEviction.json

@@ -1,6 +1,5 @@
 {
   "max_header_table_size": 128,
-  "force_huffman_on": true,
   "header_blocks":
   [
     {
@@ -28,8 +27,8 @@
         { "x-custom": "val6" }
       ],
       "encoded": [
-        "4487 6107 a4b5 8d33 ff7f 0083 ee3a 1a7e",
+        "4487 6107 a4b5 8d33 ff40 86f2 b12d 424f",
-        "83ee 3a1b 7e83 ee3a 1c"
+        "4f83 ee3a 1a7e 83ee 3a1b 7e83 ee3a 1c"
       ],
       "dynamic_table": [
         { "x-custom": "val6" },
@@ -45,8 +44,8 @@
         { "x-custom": "val3" }
       ],
       "encoded": [
-        "4487 6107 a4b5 8d33 ff7f 0083 ee3a 037e",
+        "4487 6107 a4b5 8d33 ff40 86f2 b12d 424f",
-        "83ee 3a05 7e83 ee3a 19"
+        "4f83 ee3a 037e 83ee 3a05 7e83 ee3a 19"
       ],
       "dynamic_table": [
         { "x-custom": "val3" },

codec-http2/src/test/resources/io/netty/handler/codec/http2/internal/hpack/testdata/testMaxHeaderTableSize.json

@@ -1,6 +1,5 @@
 {
   "max_header_table_size": 128,
-  "force_huffman_off": true,
   "header_blocks":
   [
     {
@@ -10,9 +9,8 @@
         { "name3": "val3" }
       ],
       "encoded": [
-        "4005 6e61 6d65 3104 7661 6c31 4005 6e61",
+        "4084 a874 943f 83ee 3a03 4084 a874 945f",
-        "6d65 3204 7661 6c32 4005 6e61 6d65 3304",
+        "83ee 3a05 4084 a874 959f 83ee 3a19"
-        "7661 6c33"
       ],
       "dynamic_table": [
         { "name3": "val3" },
@@ -28,7 +26,7 @@
         { "name2": "val2" }
       ],
       "encoded": [
-        "3f32 be40 056e 616d 6532 0476 616c 32"
+        "3f32 be40 84a8 7494 5f83 ee3a 05"
       ],
       "dynamic_table": [
         { "name2": "val2" }
@@ -43,8 +41,8 @@
         { "name3": "val3" }
       ],
       "encoded": [
-        "3f61 4005 6e61 6d65 3104 7661 6c31 bf40",
+        "3f61 4084 a874 943f 83ee 3a03 bf40 84a8",
-        "056e 616d 6533 0476 616c 33"
+        "7495 9f83 ee3a 19"
       ],
       "dynamic_table": [
         { "name3": "val3" },

codec-http2/src/test/resources/io/netty/handler/codec/http2/internal/hpack/testdata/testSpecExampleC2_1.json

@@ -1,5 +1,4 @@
 {
-  "force_huffman_off": true,
   "header_blocks":
   [
     {
@@ -7,8 +6,8 @@
         { "custom-key": "custom-header" }
       ],
       "encoded": [
-        "400a 6375 7374 6f6d 2d6b 6579 0d63 7573",
+        "4088 25a8 49e9 5ba9 7d7f 8925 a849 e95a",
-        "746f 6d2d 6865 6164 6572"
+        "728e 42d9"
       ],
       "dynamic_table": [
         { "custom-key": "custom-header" }

codec-http2/src/test/resources/io/netty/handler/codec/http2/internal/hpack/testdata/testSpecExampleC2_2.json

@@ -1,6 +1,4 @@
 {
-  "use_indexing": false,
-  "force_huffman_off": true,
   "header_blocks":
   [
     {
@@ -8,11 +6,12 @@
         { ":path": "/sample/path" }
       ],
       "encoded": [
-        "040c 2f73 616d 706c 652f 7061 7468"
+        "4489 6103 a6ba 0ac5 634c ff"
       ],
       "dynamic_table": [
+        { ":path": "/sample/path" }
       ],
-      "table_size": 0
+      "table_size": 49
     }
   ]
 }

codec-http2/src/test/resources/io/netty/handler/codec/http2/internal/hpack/testdata/testSpecExampleC2_3.json

@@ -1,6 +1,5 @@
 {
   "sensitive_headers": true,
-  "force_huffman_off": true,
   "header_blocks":
   [
     {
@@ -8,8 +7,7 @@
         { "password": "secret" }
       ],
       "encoded": [
-        "1008 7061 7373 776f 7264 0673 6563 7265",
+        "1086 ac68 4783 d927 8441 4961 53"
-        "74"
       ],
       "dynamic_table": [
       ],

codec-http2/src/test/resources/io/netty/handler/codec/http2/internal/hpack/testdata/testSpecExampleC3.json

@@ -1,5 +1,4 @@
 {
-  "force_huffman_off": true,
   "header_blocks":
   [
     {
@@ -10,8 +9,8 @@
         { ":authority": "www.example.com" }
       ],
       "encoded": [
-        "8286 8441 0f77 7777 2e65 7861 6d70 6c65",
+        "8286 8441 8cf1 e3c2 e5f2 3a6b a0ab 90f4",
-        "2e63 6f6d"
+        "ff"
       ],
       "dynamic_table": [
         { ":authority": "www.example.com" }
@@ -27,7 +26,7 @@
         { "cache-control": "no-cache" }
       ],
       "encoded": [
-        "8286 84be 5808 6e6f 2d63 6163 6865"
+        "8286 84be 5886 a8eb 1064 9cbf"
       ],
       "dynamic_table": [
         { "cache-control": "no-cache" },
@@ -44,8 +43,8 @@
         { "custom-key": "custom-value" }
       ],
       "encoded": [
-        "8287 85bf 400a 6375 7374 6f6d 2d6b 6579",
+        "8287 85bf 4088 25a8 49e9 5ba9 7d7f 8925",
-        "0c63 7573 746f 6d2d 7661 6c75 65"
+        "a849 e95b b8e8 b4bf"
       ],
       "dynamic_table": [
         { "custom-key": "custom-value" },

codec-http2/src/test/resources/io/netty/handler/codec/http2/internal/hpack/testdata/testSpecExampleC5.json

@@ -1,6 +1,5 @@
 {
   "max_header_table_size": 256,
-  "force_huffman_off": true,
   "header_blocks":
   [
     {
@@ -11,11 +10,10 @@
         { "location": "https://www.example.com" }
       ],
       "encoded": [
-        "4803 3330 3258 0770 7269 7661 7465 611d",
+        "4882 6402 5885 aec3 771a 4b61 96d0 7abe",
-        "4d6f 6e2c 2032 3120 4f63 7420 3230 3133",
+        "9410 54d4 44a8 2005 9504 0b81 66e0 82a6",
-        "2032 303a 3133 3a32 3120 474d 546e 1768",
+        "2d1b ff6e 919d 29ad 1718 63c7 8f0b 97c8",
-        "7474 7073 3a2f 2f77 7777 2e65 7861 6d70",
+        "e9ae 82ae 43d3"
-        "6c65 2e63 6f6d"
       ],
       "dynamic_table": [
         { "location": "https://www.example.com" },
@@ -53,13 +51,11 @@
         { "set-cookie": "foo=ASDJKHQKBZXOQWEOPIUAXQWEOIU; max-age=3600; version=1" }
       ],
       "encoded": [
-        "88c1 611d 4d6f 6e2c 2032 3120 4f63 7420",
+        "88c1 6196 d07a be94 1054 d444 a820 0595",
-        "3230 3133 2032 303a 3133 3a32 3220 474d",
+        "040b 8166 e084 a62d 1bff c05a 839b d9ab",
-        "54c0 5a04 677a 6970 7738 666f 6f3d 4153",
+        "77ad 94e7 821d d7f2 e6c7 b335 dfdf cd5b",
-        "444a 4b48 514b 425a 584f 5157 454f 5049",
+        "3960 d5af 2708 7f36 72c1 ab27 0fb5 291f",
-        "5541 5851 5745 4f49 553b 206d 6178 2d61",
+        "9587 3160 65c0 03ed 4ee5 b106 3d50 07"
-        "6765 3d33 3630 303b 2076 6572 7369 6f6e",
-        "3d31"
       ],
       "dynamic_table": [
         { "set-cookie": "foo=ASDJKHQKBZXOQWEOPIUAXQWEOIU; max-age=3600; version=1" },

codec-http2/src/test/resources/io/netty/handler/codec/http2/internal/hpack/testdata/testSpecExampleC6.json

@@ -1,6 +1,5 @@
 {
   "max_header_table_size": 256,
-  "force_huffman_on": true,
   "header_blocks":
   [
     {
@@ -32,7 +31,7 @@
         { "location": "https://www.example.com" }
       ],
       "encoded": [
-        "4883 640e ffc1 c0bf"
+        "4803 3330 37c1 c0bf"
       ],
       "dynamic_table": [
         { ":status": "307" },

codec-http2/src/test/resources/io/netty/handler/codec/http2/internal/hpack/testdata/testStaticTableEntries.json

@@ -1,5 +1,4 @@
 {
-  "force_huffman_on": true,
   "header_blocks":
   [
     {

microbench/src/main/java/io/netty/microbench/http2/internal/hpack/EncoderBenchmark.java

@@ -40,8 +40,11 @@ import org.openjdk.jmh.annotations.Fork;
 import org.openjdk.jmh.annotations.Level;
 import org.openjdk.jmh.annotations.Measurement;
 import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
 import org.openjdk.jmh.annotations.Param;
+import org.openjdk.jmh.annotations.Scope;
 import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
 import org.openjdk.jmh.annotations.TearDown;
 import org.openjdk.jmh.annotations.Threads;
 import org.openjdk.jmh.annotations.Warmup;
@@ -49,11 +52,14 @@ import org.openjdk.jmh.infra.Blackhole;
 
 import java.io.IOException;
 import java.util.List;
+import java.util.concurrent.TimeUnit;
 
 @Fork(1)
 @Threads(1)
+@State(Scope.Benchmark)
 @Warmup(iterations = 5)
 @Measurement(iterations = 5)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
 public class EncoderBenchmark extends AbstractMicrobenchmark {
 
     @Param
@@ -86,7 +92,7 @@ public class EncoderBenchmark extends AbstractMicrobenchmark {
     }
 
     @Benchmark
-    @BenchmarkMode(Mode.Throughput)
+    @BenchmarkMode(Mode.AverageTime)
     public void encode(Blackhole bh) throws IOException {
         Encoder encoder = new Encoder(maxTableSize);
         output.clear();