From 34fa44c3a7e73d981464e263d2b20eefdb81a955 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sun, 13 Sep 2026 16:53:57 +0100 Subject: [PATCH] perf: Huffman decode HPACK string literals into reusable scratch arrays Motivation: HuffmanDecoder collected the decoded symbols in a ByteArrayOutputStream, one synchronized write(int) per symbol, growing it from 32 bytes by doubling, and copied it out with toByteArray; Decoder then copied the result again into a String. The raw literal path allocated and copied once through InputStream.readNBytes and again into the String. This runs for every string literal of every inbound header block, which browsers Huffman code almost entirely. Modification: Decoder keeps two scratch arrays, reused across header blocks: the coded literal is read into one with readNBytes(byte[], int, int), and HuffmanDecoder.decode(buf, length, out) writes the symbols by index into the other, which the caller sizes from the static bound that the shortest code in the HPACK Huffman table is 5 bits (maxDecodedLength). The String is built straight from the scratch array with the ISO-8859-1 charset, the single copy left in either path. The arrays grow to the longest literal seen on the connection, which the max header list size and the dynamic table capacity bound. A MiMa exclude covers the package-private HuffmanDecoder signature. Result: No lock, no stream growth and one copy instead of two or three per string literal: decoding a typical browser request's header block is about 20-25% faster Huffman coded and 35-40% faster raw on JDK 17 (see the PR for the JMH figures, taken on a loaded laptop). Tests: - sbt "http-core/Test/testOnly org.apache.pekko.http.shaded.com.twitter.hpack.HuffmanDecoderSpec org.apache.pekko.http.impl.engine.http2.hpack.HpackDecoderSpec" (Scala 2.13.18 and 3.3.8) - sbt "http2-tests/Test/testOnly org.apache.pekko.http.impl.engine.http2.Http2ServerSpec org.apache.pekko.http.impl.engine.http2.Http2ClientSpec org.apache.pekko.http.impl.engine.http2.RequestParsingSpec" - sbt "http-bench-jmh/Jmh/run -f 1 -wi 3 -i 5 -w 1 -r 1 HpackDecoderBenchmark" on main and on this branch - sbt http-core/mimaReportBinaryIssues; sbt headerCheckAll; sbt javafmtCheckAll (JDK 17) - scalafmt --mode diff-ref=upstream/main References: Refs #1301 - the same change for the encoder; Refs #1251 - the InputStream decoding this builds on --- .../twitter/hpack/HpackDecoderBenchmark.scala | 78 ++++++++++++++ .../shaded/com/twitter/hpack/Decoder.java | 31 ++++-- .../com/twitter/hpack/HuffmanDecoder.java | 36 ++++--- .../hpack-huffman-decode-scratch.excludes | 19 ++++ .../engine/http2/hpack/HpackDecoderSpec.scala | 37 ++++++- .../twitter/hpack/HuffmanDecoderSpec.scala | 101 ++++++++++++++++++ 6 files changed, 281 insertions(+), 21 deletions(-) create mode 100644 http-bench-jmh/src/main/scala/org/apache/pekko/http/shaded/com/twitter/hpack/HpackDecoderBenchmark.scala create mode 100644 http-core/src/main/mima-filters/2.0.x.backwards.excludes/hpack-huffman-decode-scratch.excludes create mode 100644 http-core/src/test/scala/org/apache/pekko/http/shaded/com/twitter/hpack/HuffmanDecoderSpec.scala diff --git a/http-bench-jmh/src/main/scala/org/apache/pekko/http/shaded/com/twitter/hpack/HpackDecoderBenchmark.scala b/http-bench-jmh/src/main/scala/org/apache/pekko/http/shaded/com/twitter/hpack/HpackDecoderBenchmark.scala new file mode 100644 index 0000000000..095430fd32 --- /dev/null +++ b/http-bench-jmh/src/main/scala/org/apache/pekko/http/shaded/com/twitter/hpack/HpackDecoderBenchmark.scala @@ -0,0 +1,78 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.pekko.http.shaded.com.twitter.hpack + +import java.io.ByteArrayOutputStream +import java.util.concurrent.TimeUnit + +import org.openjdk.jmh.annotations._ + +import org.apache.pekko.util.ByteString + +@State(Scope.Benchmark) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@BenchmarkMode(Array(Mode.AverageTime)) +@Fork(1) +@Warmup(iterations = 3, time = 1) +@Measurement(iterations = 5, time = 1) +class HpackDecoderBenchmark { + + /** `default` is what a browser sends (Huffman where shorter), the other two force one string literal form. */ + @Param(Array("default", "huffman", "raw")) + var mode = "default" + + var decoder: Decoder = null + var headerBlock: ByteString = null + + // a typical browser request; the block is encoded without indexing so that every value is a string literal + // and decoding it does the same work on every call + val headers: Array[(String, String)] = Array( + ":method" -> "GET", + ":scheme" -> "https", + ":authority" -> "www.example.com", + ":path" -> "/api/v1/users/12345/orders?page=2&sort=created_at&direction=desc", + "user-agent" -> + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36", + "accept" -> "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8", + "accept-language" -> "en-US,en;q=0.9", + "accept-encoding" -> "gzip, deflate, br", + "cookie" -> "session=ab12cd34ef56ab12cd34ef56ab12cd34ef56ab12; theme=dark; consent=1", + "cache-control" -> "no-cache") + + private val listener: HeaderListener = + (_: String, _: String, parsed: AnyRef, _: Boolean) => parsed + + @Setup + def setup(): Unit = { + val encoder = mode match { + case "default" => new Encoder(4096, false, false, false) + case "huffman" => new Encoder(4096, false, true, false) + case "raw" => new Encoder(4096, false, false, true) + } + val out = new ByteArrayOutputStream() + headers.foreach { case (name, value) => encoder.encodeHeader(out, name, value, false) } + headerBlock = ByteString(out.toByteArray) + decoder = new Decoder(8192, 4096) + } + + @Benchmark + def decodeHeaders(): Unit = { + decoder.decode(headerBlock.asInputStream, listener) + decoder.endHeaderBlock() + } +} diff --git a/http-core/src/main/java/org/apache/pekko/http/shaded/com/twitter/hpack/Decoder.java b/http-core/src/main/java/org/apache/pekko/http/shaded/com/twitter/hpack/Decoder.java index 63bae0b75b..b0a07f0212 100644 --- a/http-core/src/main/java/org/apache/pekko/http/shaded/com/twitter/hpack/Decoder.java +++ b/http-core/src/main/java/org/apache/pekko/http/shaded/com/twitter/hpack/Decoder.java @@ -32,10 +32,10 @@ package org.apache.pekko.http.shaded.com.twitter.hpack; import static org.apache.pekko.http.shaded.com.twitter.hpack.HeaderField.HEADER_ENTRY_OVERHEAD; +import static org.apache.pekko.http.shaded.com.twitter.hpack.HpackUtil.ISO_8859_1; import java.io.IOException; import java.io.InputStream; -import org.apache.pekko.http.impl.util.StringTools; import org.apache.pekko.http.shaded.com.twitter.hpack.HpackUtil.IndexType; public final class Decoder { @@ -67,6 +67,12 @@ public final class Decoder { private int valueLength; private String name; + // scratch arrays for reading and Huffman decoding string literals, reused across header blocks; + // they grow to the longest literal seen on the connection, which the max header (list) size and + // the dynamic table capacity bound + private byte[] literalBuf = new byte[128]; + private byte[] decodedBuf = new byte[128]; + private enum State { READ_HEADER_REPRESENTATION, READ_MAX_DYNAMIC_TABLE_SIZE, @@ -508,21 +514,32 @@ private boolean exceedsMaxHeaderSize(long size) { } private String readStringLiteral(InputStream in, int length) throws IOException { + // read the literal into the reusable scratch array: the String constructor copies anyway, so + // a per-literal array would be a second allocation and copy + literalBuf = ensureCapacity(literalBuf, length); // readNBytes rather than read: InputStream.read(byte[]) is free to return fewer bytes than // requested even when more are available, which would be reported here as a decompression // failure - byte[] buf = in.readNBytes(length); - if (buf.length != length) { + if (in.readNBytes(literalBuf, 0, length) != length) { throw DECOMPRESSION_EXCEPTION; } - final byte[] result; if (huffmanEncoded) { - result = Huffman.DECODER.decode(buf); + decodedBuf = ensureCapacity(decodedBuf, HuffmanDecoder.maxDecodedLength(length)); + int decodedLength = Huffman.DECODER.decode(literalBuf, length, decodedBuf); + // ISO-8859-1 maps every octet to the char of the same value: string literals are opaque + // octets + return new String(decodedBuf, 0, decodedLength, ISO_8859_1); } else { - result = buf; + return new String(literalBuf, 0, length, ISO_8859_1); + } + } + + private static byte[] ensureCapacity(byte[] buf, int length) { + if (buf.length >= length) { + return buf; } - return StringTools.asciiStringFromBytes(result); + return new byte[Math.max(length, buf.length << 1)]; } private static byte readByte(InputStream in) throws IOException { diff --git a/http-core/src/main/java/org/apache/pekko/http/shaded/com/twitter/hpack/HuffmanDecoder.java b/http-core/src/main/java/org/apache/pekko/http/shaded/com/twitter/hpack/HuffmanDecoder.java index 3d338690f4..1b6e999beb 100644 --- a/http-core/src/main/java/org/apache/pekko/http/shaded/com/twitter/hpack/HuffmanDecoder.java +++ b/http-core/src/main/java/org/apache/pekko/http/shaded/com/twitter/hpack/HuffmanDecoder.java @@ -31,7 +31,6 @@ package org.apache.pekko.http.shaded.com.twitter.hpack; -import java.io.ByteArrayOutputStream; import java.io.IOException; final class HuffmanDecoder { @@ -55,20 +54,33 @@ final class HuffmanDecoder { } /** - * Decompresses the given Huffman coded string literal. + * The shortest code in the HPACK Huffman table is 5 bits long, so a coded string literal of + * n bytes decodes to at most n * 8 / 5 symbols. + */ + static int maxDecodedLength(int codedLength) { + return (int) (((long) codedLength * 8) / 5); + } + + /** + * Decompresses the given Huffman coded string literal into out, which must hold at + * least {@link #maxDecodedLength(int)} bytes for length. The symbols are written by + * index instead of through an OutputStream, whose per-symbol write(int) + * (synchronized on ByteArrayOutputStream) together with the growth and copy-out of + * that stream dominated the cost of decoding. * * @param buf the string literal to be decoded - * @return the output stream for the compressed data - * @throws IOException if an I/O error occurs. In particular, an IOException may be - * thrown if the output stream has been closed. + * @param length the number of bytes of buf that make up the string literal + * @param out the array to write the decoded symbols to, starting at index 0 + * @return the number of symbols written to out + * @throws IOException if the coded data contains the EOS symbol or is not padded with the most + * significant bits of the EOS code */ - public byte[] decode(byte[] buf) throws IOException { - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - + public int decode(byte[] buf, int length, byte[] out) throws IOException { Node node = root; int current = 0; int bits = 0; - for (int i = 0; i < buf.length; i++) { + int pos = 0; + for (int i = 0; i < length; i++) { int b = buf[i] & 0xFF; current = (current << 8) | b; bits += 8; @@ -80,7 +92,7 @@ public byte[] decode(byte[] buf) throws IOException { if (node.symbol == HpackUtil.HUFFMAN_EOS) { throw EOS_DECODED; } - baos.write(node.symbol); + out[pos++] = (byte) node.symbol; node = root; } } @@ -91,7 +103,7 @@ public byte[] decode(byte[] buf) throws IOException { node = node.children[c]; if (node.isTerminal() && node.bits <= bits) { bits -= node.bits; - baos.write(node.symbol); + out[pos++] = (byte) node.symbol; node = root; } else { break; @@ -106,7 +118,7 @@ public byte[] decode(byte[] buf) throws IOException { throw INVALID_PADDING; } - return baos.toByteArray(); + return pos; } private static final class Node { diff --git a/http-core/src/main/mima-filters/2.0.x.backwards.excludes/hpack-huffman-decode-scratch.excludes b/http-core/src/main/mima-filters/2.0.x.backwards.excludes/hpack-huffman-decode-scratch.excludes new file mode 100644 index 0000000000..82dbf91b5e --- /dev/null +++ b/http-core/src/main/mima-filters/2.0.x.backwards.excludes/hpack-huffman-decode-scratch.excludes @@ -0,0 +1,19 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# The shaded HPACK HuffmanDecoder (a package-private class) decodes into a caller-provided array +ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.pekko.http.shaded.com.twitter.hpack.HuffmanDecoder.decode") diff --git a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/http2/hpack/HpackDecoderSpec.scala b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/http2/hpack/HpackDecoderSpec.scala index 2e0d907113..d2fc04e521 100644 --- a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/http2/hpack/HpackDecoderSpec.scala +++ b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/http2/hpack/HpackDecoderSpec.scala @@ -63,9 +63,11 @@ class HpackDecoderSpec extends AnyWordSpec with Matchers { out.toByteArray } - private def decode(in: InputStream): Seq[(String, String)] = { + private def decode(in: InputStream): Seq[(String, String)] = + decode(new Decoder(maxHeaderSize, maxHeaderTableSize), in) + + private def decode(decoder: Decoder, in: InputStream): Seq[(String, String)] = { val decoded = ListBuffer.empty[(String, String)] - val decoder = new Decoder(maxHeaderSize, maxHeaderTableSize) decoder.decode(in, new HeaderListener { override def addHeader(name: String, value: String, parsed: AnyRef, sensitive: Boolean): AnyRef = { @@ -116,5 +118,36 @@ class HpackDecoderSpec extends AnyWordSpec with Matchers { // a literal header field with incremental indexing, new name, whose name length never arrives a[IOException] should be thrownBy decode(new ByteArrayInputStream(Array[Byte](0x40))) } + + "decode string literals longer than its initial scratch space, Huffman coded and raw" in { + // the decoder reads literals into reusable arrays that start at 128 bytes; a value of letters is + // Huffman coded, a value of mostly '|' (a 15-bit code) is sent raw because that is shorter + val long = Seq("x-huffman" -> ("abcdefghij" * 50), "x-raw" -> ("|" * 500)) + decode(new ByteArrayInputStream(encode(long: _*))) shouldEqual long + } + + "decode consecutive header blocks of varying literal lengths with the same decoder" in { + // a literal shorter than the previous one must not pick up the tail the previous one left in the + // reused scratch arrays + val decoder = new Decoder(maxHeaderSize, maxHeaderTableSize) + val blocks = Seq( + Seq("x-a" -> ("abcdefghij" * 40)), + Seq("x-b" -> "short", "x-c" -> ("|" * 300)), + Seq("x-d" -> "s", "x-e" -> "|"), + Seq("x-f" -> "")) + blocks.foreach { block => + // a fresh encoder per block, so no block refers to what an earlier one put in the dynamic table + decode(decoder, new ByteArrayInputStream(encode(block: _*))) shouldEqual block + } + } + + "decode a raw string literal carrying opaque octets above 0x7F to the chars of the same value" in { + // literal header field without indexing (0x00), new name: raw name "x-opaque", raw 128-octet value + val name = "x-opaque".getBytes("US-ASCII") + val value = Array.tabulate(128)(i => (0x80 + i).toByte) + // 0x7f 0x01: 7-bit prefix integer 128 = 127 + 1, Huffman flag clear + val block = Array[Byte](0x00, name.length.toByte) ++ name ++ Array[Byte](0x7F, 0x01) ++ value + decode(new ByteArrayInputStream(block)) shouldEqual Seq("x-opaque" -> new String(value, "ISO-8859-1")) + } } } diff --git a/http-core/src/test/scala/org/apache/pekko/http/shaded/com/twitter/hpack/HuffmanDecoderSpec.scala b/http-core/src/test/scala/org/apache/pekko/http/shaded/com/twitter/hpack/HuffmanDecoderSpec.scala new file mode 100644 index 0000000000..b667deb06e --- /dev/null +++ b/http-core/src/test/scala/org/apache/pekko/http/shaded/com/twitter/hpack/HuffmanDecoderSpec.scala @@ -0,0 +1,101 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.pekko.http.shaded.com.twitter.hpack + +import java.io.IOException +import java.nio.charset.StandardCharsets + +import org.scalatest.matchers.should.Matchers +import org.scalatest.wordspec.AnyWordSpec + +class HuffmanDecoderSpec extends AnyWordSpec with Matchers { + + private def bytes(hex: String): Array[Byte] = hex.grouped(2).map(Integer.parseInt(_, 16).toByte).toArray + + private def decode(coded: Array[Byte]): Array[Byte] = { + val out = new Array[Byte](HuffmanDecoder.maxDecodedLength(coded.length)) + val length = Huffman.DECODER.decode(coded, coded.length, out) + out.take(length) + } + + /** Huffman codes the octets straight from the HPACK table, independently of HuffmanEncoder. */ + private def encode(octets: Array[Byte]): Array[Byte] = { + val out = new java.io.ByteArrayOutputStream() + var current = 0L + var bits = 0 + octets.foreach { octet => + val symbol = octet & 0xFF + val nbits = HpackUtil.HUFFMAN_CODE_LENGTHS(symbol) + current = (current << nbits) | HpackUtil.HUFFMAN_CODES(symbol) + bits += nbits + while (bits >= 8) { + bits -= 8 + out.write((current >> bits).toInt) + } + } + if (bits > 0) out.write(((current << (8 - bits)) | (0xFF >>> bits)).toInt) + out.toByteArray + } + + "HuffmanDecoder" should { + "decode the string literals from RFC 7541 Appendix C.4" in { + Seq( + "f1e3c2e5f23a6ba0ab90f4ff" -> "www.example.com", + "a8eb10649cbf" -> "no-cache", + "25a849e95ba97d7f" -> "custom-key", + "25a849e95bb8e8b4bf" -> "custom-value").foreach { + case (coded, expected) => + new String(decode(bytes(coded)), StandardCharsets.ISO_8859_1) shouldEqual expected + } + } + + "decode every octet to the symbol of the same value" in { + val allOctets = Array.tabulate(256)(_.toByte) + decode(encode(allOctets)) shouldEqual allOctets + } + + "decode nothing from an empty literal" in { + decode(Array.emptyByteArray) shouldEqual Array.emptyByteArray + } + + "fill the output bound exactly for a literal made of the shortest codes" in { + // '0' has the 5-bit code 00000, so 40 coded bytes carry 64 symbols, the maxDecodedLength bound + val zeros = Array.fill[Byte](64)('0') + val coded = encode(zeros) + coded.length shouldEqual 40 + HuffmanDecoder.maxDecodedLength(coded.length) shouldEqual 64 + decode(coded) shouldEqual zeros + } + + "only read the given number of bytes of the input" in { + val coded = bytes("a8eb10649cbf") ++ Array[Byte](0, 0, 0) + val out = new Array[Byte](HuffmanDecoder.maxDecodedLength(6)) + val length = Huffman.DECODER.decode(coded, 6, out) + new String(out, 0, length, StandardCharsets.ISO_8859_1) shouldEqual "no-cache" + } + + "reject a literal containing the EOS symbol" in { + (the[IOException] thrownBy decode(bytes("ffffffff"))).getMessage shouldEqual "EOS Decoded" + } + + "reject padding that is not the most significant bits of the EOS code" in { + // 00000 decodes '0' and leaves three zero padding bits, which must have been ones + (the[IOException] thrownBy decode(bytes("00"))).getMessage shouldEqual "Invalid Padding" + } + } +}