perf: streamline HTTP/2 request header copy (iteratorCharSequence + reuse toRelativeUrl for :path)

hyperxpro · hyperxpro · commit 8f8e6db3ee96 · 2026-05-31T19:57:33.000Z
diff --git a/client/src/jmh/java/org/asynchttpclient/bench/Http2HeaderConversionBenchmark.java b/client/src/jmh/java/org/asynchttpclient/bench/Http2HeaderConversionBenchmark.java
@@ -0,0 +1,138 @@
+/*
+ *    Copyright (c) 2026 AsyncHttpClient Project. All rights reserved.
+ *
+ *    Licensed 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
+ */
+package org.asynchttpclient.bench;
+
+import io.netty.handler.codec.http.DefaultHttpHeaders;
+import io.netty.handler.codec.http.HttpHeaderNames;
+import io.netty.handler.codec.http.HttpHeaders;
+import io.netty.handler.codec.http2.DefaultHttp2Headers;
+import io.netty.handler.codec.http2.Http2Headers;
+import io.netty.util.AsciiString;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Measures the per-request HTTP/2 header conversion done in
+ * {@code NettyRequestSender.sendHttp2Frames}: iterating the HTTP/1.1
+ * {@link HttpHeaders} and copying each into {@link Http2Headers}, skipping
+ * connection-specific names.
+ *
+ * <p>BASELINE reproduces the production code exactly:
+ * {@code headers.forEach(e -> { String n = e.getKey().toLowerCase(); if (!STRING_SET.contains(n)) h2.add(n, e.getValue()); })}.
+ * The {@code forEach} forces every stored {@link AsciiString} name into a
+ * {@link String}, then {@code toLowerCase()} allocates a second {@link String}.
+ *
+ * <p>PROPOSED uses {@link HttpHeaders#iteratorCharSequence()} (no String
+ * conversion) and matches against the {@link HttpHeaderNames}
+ * {@link AsciiString} constants case-insensitively. It still lowercases each
+ * forwarded name (HTTP/2 requires lowercase names and Netty's validating
+ * {@link DefaultHttp2Headers} throws on an uppercase name rather than
+ * normalising it), but allocation-free for already-lowercase names since
+ * {@link AsciiString#toLowerCase()} / {@link String#toLowerCase(Locale)}
+ * return the same instance when there is nothing to change. The header set below
+ * therefore includes mixed-case user-supplied names to exercise that path.
+ */
+@State(Scope.Thread)
+@BenchmarkMode(Mode.AverageTime)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+public class Http2HeaderConversionBenchmark {
+
+    private static final Set<String> EXCLUDED_STRING = new HashSet<>(Arrays.asList(
+            "connection", "keep-alive", "proxy-connection", "transfer-encoding", "upgrade", "host"));
+
+    private static final Set<AsciiString> EXCLUDED_ASCII = new HashSet<>(Arrays.asList(
+            HttpHeaderNames.CONNECTION, HttpHeaderNames.KEEP_ALIVE, HttpHeaderNames.PROXY_CONNECTION,
+            HttpHeaderNames.TRANSFER_ENCODING, HttpHeaderNames.UPGRADE, HttpHeaderNames.HOST));
+
+    private HttpHeaders headers;
+
+    @Setup
+    public void setup() {
+        // Representative request header set built the way NettyRequestFactory builds it:
+        // names are AsciiString constants from HttpHeaderNames.
+        headers = new DefaultHttpHeaders(false);
+        headers.set(HttpHeaderNames.HOST, "www.example.com");
+        headers.set(HttpHeaderNames.USER_AGENT, "AHC/3.0");
+        headers.set(HttpHeaderNames.ACCEPT, "*/*");
+        headers.set(HttpHeaderNames.ACCEPT_ENCODING, "gzip, deflate");
+        headers.set(HttpHeaderNames.CONTENT_TYPE, "application/json; charset=utf-8");
+        headers.set(HttpHeaderNames.CONTENT_LENGTH, "256");
+        headers.set(HttpHeaderNames.AUTHORIZATION, "Bearer abcdef0123456789");
+        headers.set(HttpHeaderNames.COOKIE, "session=deadbeef; theme=dark");
+        headers.set(HttpHeaderNames.CONNECTION, "keep-alive");
+        headers.set(HttpHeaderNames.CACHE_CONTROL, "no-cache");
+        // Mixed-case user-supplied names (stored as String, original casing preserved) — these are the
+        // names the proposed path must lowercase, since validating DefaultHttp2Headers rejects uppercase.
+        headers.add("X-Request-ID", "abc-123-def-456");
+        headers.add("X-Custom-Header", "some-custom-value");
+    }
+
+    /** Exact reproduction of production NettyRequestSender.sendHttp2Frames header loop. */
+    @Benchmark
+    public Http2Headers baseline_forEach_toLowerCase() {
+        Http2Headers h2 = new DefaultHttp2Headers()
+                .method("GET").path("/path?q=1").scheme("https").authority("www.example.com");
+        for (Map.Entry<String, String> entry : headers) {
+            String name = entry.getKey().toLowerCase();
+            if (!EXCLUDED_STRING.contains(name)) {
+                h2.add(name, entry.getValue());
+            }
+        }
+        return h2;
+    }
+
+    /** Proposed: iteratorCharSequence + AsciiString-keyed case-insensitive skip set. */
+    @Benchmark
+    public Http2Headers proposed_charSequence_ascii() {
+        Http2Headers h2 = new DefaultHttp2Headers()
+                .method("GET").path("/path?q=1").scheme("https").authority("www.example.com");
+        Iterator<Map.Entry<CharSequence, CharSequence>> it = headers.iteratorCharSequence();
+        while (it.hasNext()) {
+            Map.Entry<CharSequence, CharSequence> entry = it.next();
+            CharSequence name = entry.getKey();
+            if (!containsIgnoreCase(name)) {
+                h2.add(toLowerCaseName(name), entry.getValue());
+            }
+        }
+        return h2;
+    }
+
+    /** Mirrors production NettyRequestSender.toLowerCaseHeaderName — allocation-free when already lowercase. */
+    private static CharSequence toLowerCaseName(CharSequence name) {
+        if (name instanceof AsciiString) {
+            return ((AsciiString) name).toLowerCase();
+        }
+        return name.toString().toLowerCase(Locale.ROOT);
+    }
+
+    private static boolean containsIgnoreCase(CharSequence name) {
+        // Direct constant comparison: HTTP/2 forbids exactly these 6 connection-specific names.
+        // AsciiString.contentEqualsIgnoreCase short-circuits on length mismatch (cheap).
+        return HttpHeaderNames.CONNECTION.contentEqualsIgnoreCase(name)
+                || HttpHeaderNames.HOST.contentEqualsIgnoreCase(name)
+                || HttpHeaderNames.TRANSFER_ENCODING.contentEqualsIgnoreCase(name)
+                || HttpHeaderNames.UPGRADE.contentEqualsIgnoreCase(name)
+                || HttpHeaderNames.KEEP_ALIVE.contentEqualsIgnoreCase(name)
+                || HttpHeaderNames.PROXY_CONNECTION.contentEqualsIgnoreCase(name);
+    }
+}
diff --git a/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java b/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java
@@ -24,6 +24,7 @@
 import io.netty.channel.ChannelPromise;
 import io.netty.handler.codec.http.DefaultFullHttpRequest;
 import io.netty.handler.codec.http.DefaultHttpHeaders;
+import io.netty.handler.codec.http.HttpHeaderNames;
 import io.netty.handler.codec.http.HttpHeaderValues;
 import io.netty.handler.codec.http.HttpHeaders;
 import io.netty.handler.codec.http.HttpMethod;
@@ -36,6 +37,7 @@
 import io.netty.handler.codec.http2.Http2StreamChannelBootstrap;
 import io.netty.resolver.AddressResolver;
 import io.netty.resolver.AddressResolverGroup;
+import io.netty.util.AsciiString;
 import io.netty.util.ReferenceCountUtil;
 import io.netty.util.Timer;
 import io.netty.util.concurrent.Future;
@@ -81,13 +83,14 @@
 import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
+import java.util.Iterator;
 import java.util.List;
-import java.util.Set;
+import java.util.Locale;
+import java.util.Map;
 
 import static io.netty.handler.codec.http.HttpHeaderNames.EXPECT;
 import static java.util.Collections.singletonList;
 import static java.util.Objects.requireNonNull;
-import static java.util.Set.of;
 import static org.asynchttpclient.util.AuthenticatorUtils.perConnectionAuthorizationHeader;
 import static org.asynchttpclient.util.AuthenticatorUtils.perConnectionProxyAuthorizationHeader;
 import static org.asynchttpclient.util.HttpConstants.Methods.CONNECT;
@@ -420,12 +423,42 @@ private <T> NettyResponseFuture<T> newNettyResponseFuture(Request request, Async
     }
 
     /**
-     * HTTP/2 connection-specific headers that must NOT be forwarded as per RFC 7540 §8.1.2.2.
-     * These are HTTP/1.1 connection-specific headers that have no meaning in HTTP/2.
+     * Whether {@code name} is a connection-specific HTTP/1.1 header that must NOT be forwarded over
+     * HTTP/2 (RFC 7540 §8.1.2.2). These have no meaning in HTTP/2 and some servers RST the stream if
+     * they are present.
+     * <p>
+     * Matched case-insensitively against the {@link HttpHeaderNames} {@link AsciiString}
+     * constants so the per-request header copy needs no {@link String}/{@code toLowerCase} allocation;
+     * the length-mismatch short-circuit in {@code contentEqualsIgnoreCase} keeps this chain cheap and
+     * branch-predictable.
      */
-    private static final Set<String> HTTP2_EXCLUDED_HEADERS = of(
-            "connection", "keep-alive", "proxy-connection", "transfer-encoding", "upgrade", "host"
-    );
+    private static boolean isHttp2ExcludedHeader(CharSequence name) {
+        return HttpHeaderNames.CONNECTION.contentEqualsIgnoreCase(name)
+                || HttpHeaderNames.HOST.contentEqualsIgnoreCase(name)
+                || HttpHeaderNames.TRANSFER_ENCODING.contentEqualsIgnoreCase(name)
+                || HttpHeaderNames.UPGRADE.contentEqualsIgnoreCase(name)
+                || HttpHeaderNames.KEEP_ALIVE.contentEqualsIgnoreCase(name)
+                || HttpHeaderNames.PROXY_CONNECTION.contentEqualsIgnoreCase(name);
+    }
+
+    /**
+     * Lower-cases an HTTP/1.1 header name for use as an HTTP/2 header name, allocating nothing when the
+     * name is already lowercase.
+     * <p>
+     * HTTP/2 requires lowercase header field names (RFC 7540 §8.1.2) and Netty's validating
+     * {@link DefaultHttp2Headers} throws {@link io.netty.handler.codec.http2.Http2Exception} on any name
+     * containing an uppercase ASCII letter, so mixed-case user-supplied names must be normalised before
+     * they are added. Both {@link AsciiString#toLowerCase()} and
+     * {@link String#toLowerCase(Locale)} return the same instance when there is nothing to change, so the
+     * common case — AHC's own names built from lowercase {@link HttpHeaderNames} constants — allocates
+     * nothing; only genuinely mixed-case names allocate, exactly as the previous {@code toLowerCase()} did.
+     */
+    private static CharSequence toLowerCaseHeaderName(CharSequence name) {
+        if (name instanceof AsciiString) {
+            return ((AsciiString) name).toLowerCase();
+        }
+        return name.toString().toLowerCase(Locale.ROOT);
+    }
 
     public <T> void writeRequest(NettyResponseFuture<T> future, Channel channel) {
         // if the channel is dead because it was pooled and the remote server decided to close it,
@@ -575,21 +608,31 @@ private <T> void sendHttp2Frames(NettyResponseFuture<T> future, Http2StreamChann
         Uri uri = future.getUri();
 
         try {
-            // Build HTTP/2 pseudo-headers + regular headers
+            // Build HTTP/2 pseudo-headers + regular headers.
+            // :path reuses the origin-form request-target already computed for HTTP/1.1 via the pooled
+            // StringBuilder (Uri.toRelativeUrl), instead of re-concatenating path + "?" + query here.
             Http2Headers h2Headers = new DefaultHttp2Headers()
                     .method(httpRequest.method().name())
-                    .path(uri.getNonEmptyPath() + (uri.getQuery() != null ? "?" + uri.getQuery() : ""))
+                    .path(uri.toRelativeUrl())
                     .scheme(uri.getScheme())
                     .authority(hostHeader(uri));
 
             // Copy HTTP/1.1 headers, skipping connection-specific ones that are forbidden in HTTP/2.
-            // RFC 7540 §8.1.2 requires all header field names to be lowercase in HTTP/2.
-            httpRequest.headers().forEach(entry -> {
-                String name = entry.getKey().toLowerCase();
-                if (!HTTP2_EXCLUDED_HEADERS.contains(name)) {
-                    h2Headers.add(name, entry.getValue());
+            // RFC 7540 §8.1.2 requires all header field names to be lowercase in HTTP/2, and Netty's
+            // validating DefaultHttp2Headers REJECTS (throws Http2Exception) any name containing an
+            // uppercase ASCII letter — it does NOT normalise — so user-supplied mixed-case names must
+            // be lowercased here. iteratorCharSequence() yields the stored CharSequence names without
+            // the per-name String materialisation the String-typed iterator forces; toLowerCaseHeaderName
+            // then returns the name unchanged (no allocation) when it is already lowercase, which is the
+            // common case since AHC sets its own names from lowercase HttpHeaderNames.* constants.
+            Iterator<Map.Entry<CharSequence, CharSequence>> it = httpRequest.headers().iteratorCharSequence();
+            while (it.hasNext()) {
+                Map.Entry<CharSequence, CharSequence> entry = it.next();
+                CharSequence name = entry.getKey();
+                if (!isHttp2ExcludedHeader(name)) {
+                    h2Headers.add(toLowerCaseHeaderName(name), entry.getValue());
                 }
-            });
+            }
 
             // Determine if we have a body to write.
             // Support both DefaultFullHttpRequest (inline content) and NettyDirectBody (byte array/buffer bodies).
diff --git a/client/src/test/java/org/asynchttpclient/BasicHttp2Test.java b/client/src/test/java/org/asynchttpclient/BasicHttp2Test.java
@@ -262,14 +262,21 @@ private void sendEchoResponse(ChannelHandlerContext ctx, ByteBuf body, String fu
                 responseHeaders.add("x-querystring", queryString);
             }
 
-            // Echo request headers as X-{name}
+            // Echo request headers as X-{name}, and also report the exact (case-preserving) set of
+            // received non-pseudo header names in a single value so wire casing / exclusion is testable.
+            StringBuilder receivedNames = new StringBuilder();
             for (Map.Entry<CharSequence, CharSequence> entry : requestHeaders) {
                 String name = entry.getKey().toString();
                 // Skip pseudo-headers
                 if (!name.startsWith(":")) {
                     responseHeaders.add("x-" + name, entry.getValue());
+                    if (receivedNames.length() > 0) {
+                        receivedNames.append(',');
+                    }
+                    receivedNames.append(name);
                 }
             }
+            responseHeaders.add("x-received-names", receivedNames.toString());
 
             // Handle OPTIONS
             if ("OPTIONS".equalsIgnoreCase(method)) {
@@ -768,6 +775,51 @@ public void getWithHeadersOverHttp2() throws Exception {
         }
     }
 
+    /**
+     * Regression guard for the HTTP/2 header-copy rewrite (proposal 019): a user-supplied mixed-case
+     * header name must be lowercased before it reaches the validating {@link DefaultHttp2Headers}, which
+     * otherwise throws {@code Http2Exception: invalid header name}. Asserts (a) the request succeeds,
+     * (b) the name is lowercase on the wire, and (c) connection-specific names are not forwarded.
+     */
+    @Test
+    public void mixedCaseHeaderIsLowercasedAndConnectionHeadersExcludedOverHttp2() throws Exception {
+        try (AsyncHttpClient client = http2Client()) {
+            Response response = client.prepareGet(httpsUrl("/echo"))
+                    .addHeader("X-Mixed-Case", "v1")
+                    .addHeader("Another-Custom-Header", "v2")
+                    .addHeader("connection", "keep-alive")
+                    .addHeader("Keep-Alive", "timeout=5")
+                    .addHeader("Upgrade", "h2c")
+                    .execute()
+                    .get(30, SECONDS);
+
+            assertEquals(200, response.getStatusCode());
+
+            // x-received-names reports the exact, case-preserving names the server decoded off the wire.
+            String received = response.getHeader("x-received-names");
+            assertNotNull(received, "server should report received header names");
+            List<String> names = Arrays.asList(received.split(","));
+
+            // (b) mixed-case user headers arrive lowercase on the wire (case-sensitive membership check)
+            assertTrue(names.contains("x-mixed-case"),
+                    "mixed-case header should be lowercase on the wire, got: " + received);
+            assertTrue(names.contains("another-custom-header"),
+                    "mixed-case header should be lowercase on the wire, got: " + received);
+            assertFalse(names.contains("X-Mixed-Case"),
+                    "uppercase header name must not appear on the wire, got: " + received);
+
+            // (c) connection-specific names (any casing) are dropped, not forwarded as regular headers
+            for (String forbidden : new String[]{"connection", "keep-alive", "upgrade", "host"}) {
+                assertFalse(names.contains(forbidden),
+                        "connection-specific header '" + forbidden + "' must be excluded, got: " + received);
+            }
+
+            // and the values still round-trip for the forwarded headers
+            assertEquals("v1", response.getHeader("X-x-mixed-case"));
+            assertEquals("v2", response.getHeader("X-another-custom-header"));
+        }
+    }
+
     @Test
     public void postWithHeadersAndFormParamsOverHttp2() throws Exception {
         try (AsyncHttpClient client = http2Client()) {
diff --git a/client/src/test/java/org/asynchttpclient/uri/UriTest.java b/client/src/test/java/org/asynchttpclient/uri/UriTest.java
@@ -352,4 +352,27 @@ public void testGetPathWhenPathIsEmpty() {
         Uri uri = Uri.create("http://stackoverflow.com");
         assertEquals("/", uri.getNonEmptyPath(), "Incorrect path returned from getNonEmptyPath");
     }
+
+    /**
+     * The HTTP/2 writer builds the {@code :path} pseudo-header via {@link Uri#toRelativeUrl()} instead of
+     * the older {@code getNonEmptyPath() + (query != null ? "?" + query : "")} concatenation. This locks
+     * in that the two are byte-identical for representative origin-form request targets, so the switch is
+     * not an observable wire change (see proposal 021).
+     */
+    @RepeatedIfExceptionsTest(repeats = 5)
+    public void testToRelativeUrlMatchesLegacyPathConcat() {
+        for (String url : new String[]{
+                "http://example.com",                 // empty path
+                "http://example.com/",                // root path
+                "http://example.com/a/b",             // path, no query
+                "http://example.com/a/b?x=1&y=2",     // path + query
+                "http://example.com/?q=1",            // root path + query
+                "http://example.com/search?q=a%20b&n=1" // encoded query
+        }) {
+            Uri uri = Uri.create(url);
+            String legacy = uri.getNonEmptyPath() + (uri.getQuery() != null ? "?" + uri.getQuery() : "");
+            assertEquals(legacy, uri.toRelativeUrl(),
+                    "toRelativeUrl() must equal the legacy :path concatenation for " + url);
+        }
+    }
 }
