On Wed, 29 Apr 2026 16:53:56 GMT, Ashay Rane <[email protected]> wrote:
>> Prior to this patch, every HTTP request created a new 16KB buffer for >> encoding the header, which is typically only a few hundred bytes long. >> This increased pressure on the garbage collector when the client created >> lots of requests. This patch instead makes the header encoder reuse the >> buffer that is created during the handling of the first request. >> >> The caveat, however, is that the downstream consumers of the header are >> asynchronous, so the encoder needs to take special care to ensure that >> it doesn't modify or invalidate the buffer after it hands the buffer >> over to the downstream asynchronous pipeline. To resolve this, this >> patch snapshots the buffer data into compact copies sized to the actual >> encoded length. Doing so makes the buffer immediately available for >> reuse via `clear()` and `limit()`. >> >> For typical requests, this reduces per-request allocation from 16KB to >> a few hundred bytes (i.e. the size of the compact copy of the encoded >> headers), with the 16KB encoding buffer allocated once per connection >> instead of once per request. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Ashay Rane has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains four additional commits since > the last revision: > > - Address PR suggestions > > 1. Added an assertion to check whether sendlock is held by the current > thread. > > 2. Replaced lambda (`captureAndAddtoBuffers`) with a private static > method (`copyBuffer`) since the lambda doesn't really save us any > significant lines of code. > > 3. Used `${test.main.class}` instead of explicitly spelling out the > class name. > > 4. Used junit instead of JDK test library assertions, updated the rest > of the test accordingly. > > 5. Used `HttpTestServer` and `URIBuilder` in the test, dropped the > initial warmup since the HTTP version is "2" by construction. > > 6. Used junit's `assertSame` method for ensuring that the connection > object and the cache header buffer are the same across various > `send()` invocations. > > 7. Minor fixes (updated copyright year and comments) > - Merge branch 'master' into JDK-8383248-reuse-buffer-in-header-encoding > - Merge branch 'master' of https://github.com/openjdk/jdk into > JDK-8383248-reuse-buffer-in-header-encoding > - Reuse buffer for encoding headers instead of allocating one per request > > Prior to this patch, every HTTP request created a new 16KB buffer for > encoding the header, which are typically only a few hundred bytes long. > This increased pressure on the garbage collector when the client created > lots of requests. This patch instead makes the header encoder reuse the > buffer that is created during the handling of the first request. > > The caveat, however, is that the downstream consumers of the header are > asynchronous, so the encoder needs to take special care to ensure that > it doesn't modify or invalidate the buffer after it hands the buffer > over to the downstream asynchronous pipeline. To resolve this, this > patch snapshots the buffer data into compact copies sized to the actual > encoded length. Doing so makes the buffer immediately available for > reuse via `clear()` and `limit()`. > > For typical requests, this reduces per-request allocation from ~16KB to > a few hundred bytes (i.e. the size of the compact copy of the encoded > headers), with the 16KB encoding buffer allocated once per connection > instead of once per request. test/jdk/java/net/httpclient/http2/HeaderEncodingBufferReuseTest.java line 89: > 87: .build()) { > 88: > 89: // Force a large cached header buffer by sending 300 headers. Since we're not using `https` here the first request will be an upgrade request going through HTTP/1.1. So no buffer will be cached here. You could consider sending a regular GET/HEAD request to perform the upgrade first, and check that the version in the returned response is HTTP/2 before sending your large headers, or use `https` instead of `http`. test/jdk/java/net/httpclient/http2/HeaderEncodingBufferReuseTest.java line 116: > 114: field.setAccessible(true); > 115: return field.get(obj); > 116: } There are other ways than using reflection to get access to internals. Usually we prefer to inject some public test accessor class at runtime in the internal package. The advantage is that the injected classes get compiled by jtreg so you get a compilation error if the API changes, and it usually get changed along with the API it accesses if you use `refactor` in the IDE. That said - I'm OK with using reflection here. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30931#discussion_r3172900729 PR Review Comment: https://git.openjdk.org/jdk/pull/30931#discussion_r3172918344
