(pekko-http) branch main updated: perf: decode parser byte ranges in bulk instead of char by char (#1229)

[fa...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

pjfanning pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pekko-http.git


The following commit(s) were added to refs/heads/main by this push:
     new e202ffd09 perf: decode parser byte ranges in bulk instead of char by char (#1229)
e202ffd09 is described below

commit e202ffd09197dca9c068cfae0cb02cc9f689f601
Author: PJ Fanning <pj...@users.noreply.github.com>
AuthorDate: Fri Sep 11 09:55:09 2026 +0100

    perf: decode parser byte ranges in bulk instead of char by char (#1229)
    
    Motivation:
    asciiString built its String one character at a time through indexed ByteString
    access and a StringBuilder. Indexed access into a multi-fragment ByteString walks
    the fragment list on every byte, and the loop cannot use the JDK's bulk decode.
    ByteStringParserInput.sliceString already does the fast thing for the same job.
    
    It also sign extended: input(ix).toChar turns byte 0xC3 into U+FFC3 rather than
    U+00C3. Three of the call sites can see bytes above 0x7F - the response reason
    phrase, chunk extensions, and header names when
    illegal-response-header-name-processing-mode is warn or ignore - so those ranges
    were decoded into replacement-looking garbage. The two call sites in
    scanHeaderValue cannot: they only ever cover a range already checked to be legal
    7-bit ASCII, where the old and new behaviour agree.
    
    Modification:
    asciiString now slices and calls decodeString(ISO-8859-1), matching
    ByteStringParserInput.sliceString. Reuses the existing ISO88591 constant from
    pekko.http.impl.util.
    
    Result:
    One bulk decode instead of a per-byte loop, and byte ranges above 0x7F decode to
    the Latin-1 character rather than a sign extended one. Measured over 3M
    extractions of a 4-character header name from a request-sized ByteString: 234 ms
    before, 167 ms after.
    
    Tests:
    - sbt "http-core / Test / testOnly org.apache.pekko.http.impl.engine.parsing.*" - 242 passed
    - New ResponseParserSpec case parses a non-ASCII header name with illegal names ignored; it fails before this change, where the name comes out as f\uFFC3\uFFB6o
    - sbt http-core/mimaReportBinaryIssues - clean
    - scalafmt --mode diff-ref=upstream/main - clean
    
    References:
    None - found while auditing ByteString usage across the code base
---
 .../apache/pekko/http/impl/engine/parsing/package.scala   | 15 ++++++++-------
 .../http/impl/engine/parsing/ResponseParserSpec.scala     | 14 ++++++++++++++
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/package.scala b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/package.scala
index bc42944fd..92b2036fb 100644
--- a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/package.scala
+++ b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/package.scala
@@ -13,14 +13,13 @@
 
 package org.apache.pekko.http.impl.engine
 
-import java.lang.{ StringBuilder => JStringBuilder }
 import org.apache.pekko
 import pekko.http.scaladsl.settings.ParserSettings
 
-import scala.annotation.tailrec
 import pekko.event.LoggingAdapter
 import pekko.util.ByteString
 import pekko.http.scaladsl.model.{ ErrorInfo, StatusCode, StatusCodes }
+import pekko.http.impl.util.ISO88591
 import pekko.http.impl.util.SingletonException
 
 /**
@@ -41,11 +40,13 @@ package object parsing {
   private[http] def byteAt(input: ByteString, ix: Int): Byte =
     if (ix < input.length) input(ix) else throw NotEnoughDataException
 
-  private[http] def asciiString(input: ByteString, start: Int, end: Int): String = {
-    @tailrec def build(ix: Int = start, sb: JStringBuilder = new JStringBuilder(end - start)): String =
-      if (ix == end) sb.toString else build(ix + 1, sb.append(input(ix).toChar))
-    if (start == end) "" else build()
-  }
+  /**
+   * Decodes the given range as a String, one character per byte. Bytes above 0x7F are decoded as
+   * ISO-8859-1, as [[pekko.http.impl.util.ByteStringParserInput.sliceString]] already does; most
+   * callers have validated the range as 7-bit ASCII, for which the two agree.
+   */
+  private[http] def asciiString(input: ByteString, start: Int, end: Int): String =
+    if (start == end) "" else input.slice(start, end).decodeString(ISO88591)
 
   private[http] def logParsingError(info: ErrorInfo, log: LoggingAdapter,
       settings: ParserSettings.ErrorLoggingVerbosity,
diff --git a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/ResponseParserSpec.scala b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/ResponseParserSpec.scala
index 470ba6a2d..cf0ef5acb 100644
--- a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/ResponseParserSpec.scala
+++ b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/ResponseParserSpec.scala
@@ -103,6 +103,20 @@ abstract class ResponseParserSpec(mode: String, newLine: String) extends PekkoSp
         closeAfterResponseCompletion shouldEqual Seq(false)
       }
 
+      "a response with a non-ASCII header name, when illegal header names are ignored" in new Test {
+        override def parserSettings: ParserSettings =
+          super.parserSettings.withIllegalResponseHeaderNameProcessingMode(
+            ParserSettings.IllegalResponseHeaderNameProcessingMode.Ignore)
+
+        // a header name that is not 7-bit ASCII is an opaque byte range, decoded one character per
+        // byte (ISO-8859-1) rather than sign extended into \uFFxx characters
+        val name = new String("f\u00f6o".getBytes(java.nio.charset.StandardCharsets.UTF_8),
+          java.nio.charset.StandardCharsets.ISO_8859_1)
+        s"HTTP/1.1 200 OK${newLine}föo: bar${newLine}Content-Length: 0${newLine}${newLine}" should parseTo(
+          HttpResponse(headers = List(RawHeader(name, "bar"))))
+        closeAfterResponseCompletion shouldEqual Seq(false)
+      }
+
       "a response with a missing reason phrase" in new Test {
         s"HTTP/1.1 404 ${newLine}Content-Length: 0${newLine}${newLine}" should parseTo(HttpResponse(NotFound))
         closeAfterResponseCompletion shouldEqual Seq(false)


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pekko.apache.org
For additional commands, e-mail: commits-help@pekko.apache.org