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

hyperxpro · hyperxpro · commit 93a0798f0f04 · 2026-05-31T20:13:47.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,126 @@
+/*
+ *    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
+ *
+ *    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.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 copy in {@code NettyRequestSender.sendHttp2Frames}: the
+ * baseline (String-typed {@code forEach} + {@code toLowerCase()} + {@code HashSet} skip-set) versus
+ * {@link HttpHeaders#iteratorCharSequence()} + {@link AsciiString#contentEqualsIgnoreCase}. Both still
+ * lowercase forwarded names (Netty's validating {@link DefaultHttp2Headers} rejects uppercase), so the
+ * header set includes mixed-case 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 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,33 @@ 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 header forbidden in HTTP/2 (RFC 7540 §8.1.2.2).
+     * Matched case-insensitively against the {@link HttpHeaderNames} {@link AsciiString} constants, so the
+     * per-request header copy needs no {@link String}/{@code toLowerCase} allocation to run this check.
      */
-    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 HTTP/2, allocating nothing when it is already lowercase.
+     * Netty's validating {@link DefaultHttp2Headers} throws {@link io.netty.handler.codec.http2.Http2Exception}
+     * on a name with any uppercase ASCII letter (it does not normalise), so mixed-case user names must be
+     * lowercased before they are added. {@link AsciiString#toLowerCase()} and {@link String#toLowerCase(Locale)}
+     * both return the same instance when nothing changes, so already-lowercase names — AHC's own
+     * {@link HttpHeaderNames} constants — allocate nothing.
+     */
+    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 +599,25 @@ 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 Uri.toRelativeUrl() (pooled
+            // StringBuilder) instead of re-concatenating path + "?" + query on every request.
             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());
+            // Copy the HTTP/1.1 headers, dropping connection-specific names forbidden in HTTP/2 (RFC 7540
+            // §8.1.2.2). iteratorCharSequence() avoids the per-name String the String-typed iterator forces;
+            // see isHttp2ExcludedHeader and toLowerCaseHeaderName for the skip-check and lowercasing rules.
+            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: 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,26 @@ 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 (no wire change).
+     */
+    @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);
+        }
+    }
 }
