Mark an HTTP/2 connection instead of looking it up

[pavel-ptashyts]

Problem

ChannelManager.isHttp2(channel) asks the pipeline for the multiplex handler by name:

return channel.pipeline().get(HTTP2_MULTIPLEX) != null;

The write path asks it of every request, twice: once in sendRequestWithOpenChannel to decide
whether to store the per-request future on the channel, once in writeRequest to route the
write. DefaultChannelPipeline.get(String) walks the handler chain comparing names, and an
HTTP/1.1 connection - which has no such handler - is walked to the end to answer no. That is the
common case for anyone who has not turned HTTP/2 on.

In a profile of a client running with setHttp2Enabled(false),
DefaultChannelPipeline.context0 accounted for 83 CPU samples on this alone.

Change

The multiplex handler is installed in exactly one place, upgradePipelineToHttp2, so a channel
attribute is set beside it and isHttp2 reads that instead:

public static boolean isHttp2(Channel channel) {
    return channel.hasAttr(HTTP2_CONNECTION_ATTRIBUTE);
}

A binary search over integer keys in a small array, rather than a walk with a string compare per
handler. hasAttr rather than attr(...).get(): the latter would add an entry to the attribute
map of every HTTP/1.1 channel just to find nothing in it.

Behaviour is unchanged in every configuration, including the ones a config check would get wrong

• see below.

Why not check the config instead

Reading config.isHttp2Enabled() first would be cheaper still, and it was the first thing tried.
It is not safe: the two can disagree.

NettyConnectListener upgrades the pipeline on the ALPN result alone, not on the config:

boolean http2Negotiated = ApplicationProtocolNames.HTTP_2.equals(alpnProtocol);
if (http2Negotiated && !uri.isWebSocket()) {
    channelManager.upgradePipelineToHttp2(channel.pipeline());

And ALPN can select h2 with the flag off. DefaultSslEngineFactory advertises h2 only when
isHttp2Enabled(), but it leaves a caller-supplied SslContext alone
(config.getSslContext() != null || !config.isHttp2Enabled()), and a caller-supplied
SslEngineFactory is free to advertise whatever it likes - which the WebSocket guard beside the
upgrade already accounts for in as many words: "this guard is the backstop for a custom
SslEngineFactory that still advertises h2". upgradePipelineToHttp2AfterProxyConnect is gated on
ALPN the same way.

With http2Enabled(false) and such a context, a config check would route a genuine HTTP/2
connection down the HTTP/1.1 branch and write an HTTP/1.1 request onto an HTTP/2 pipeline. The
attribute costs nothing more than the config read would have saved, and cannot disagree with the
handler, being set where the handler is.

If HTTP/2 negotiated against http2Enabled(false) is considered unsupported, a config
short-circuit could be layered on top of this - but that is a behaviour decision rather than a
micro-optimisation, so it is not made here.

Can the attribute go stale

No. Nothing removes HTTP2_MULTIPLEX from a pipeline - the only reference to it besides the
lookup is the addLast in the upgrade - so there is no downgrade for the two to diverge across.
Stream child channels carry neither the handler nor the attribute, so isHttp2 answers no for
them exactly as it did before.

Tests

ChannelManagerHttp2MarkerTest pins the attribute to the handler: either both say HTTP/2 or
neither does, so a later change to the upgrade cannot set one and forget the other. Three cases -
a connection never upgraded, one upgraded, and a stream channel.

Checked by mutation rather than assumption: dropping the attribute assignment fails that test and
46 of the 52 in BasicHttp2Test, the write path having routed HTTP/2 connections down the
HTTP/1.1 branch.

Verification

mvnw clean verify - BUILD SUCCESS, 1488 tests, 0 failures, 0 errors, 26 skipped. Error Prone,
NullAway and Revapi all clean, with no revapi entries: isHttp2 keeps its signature and the
attribute key is private.

Caveat on the testing gate: AGENTS.md requires the build to run on JDK 11 and no JDK 11 is
installed on this machine, so it was run on JDK 17 (also in the CI matrix). The JDK 11 legs of
CI on this PR are the real gate.

Claude Code on behalf of @pavel-ptashyts

🤖 Generated with Claude Code