On Tue, 26 May 2026 20:14:14 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 incrementally with one additional > commit since the last revision: > > Address PR comments > > 1. Mark newly-introduced package private functions (`getConnections()` > and `getCachedHeaderBuffer()`) so that it's clear that they're used > only by the tests. > > 2. Early return in `getHeaderBuffer()` when `cachedHeaderBuffer` is > newly allocated, thus skipping the `clear()` and `limit()` calls. Thank you for the updates so far. The copyright year on some of these updated files will need an update. src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java line 1618: > 1616: private ByteBuffer cachedHeaderBuffer; > 1617: > 1618: // `getCachedHeaderBuffer()` is used only by tests and it should > not be Nit - the markdown style quotes aren't necessary for these comments. But if there are no more updates in this PR, then it's OK to leave them in the current form. test/jdk/java/net/httpclient/http2/HeaderEncodingBufferReuseTest.java line 128: > 126: assertEquals(1, connections.size()); > 127: assertSame(conn, connections.iterator().next()); > 128: assertSame(cached, > HttpClientImplAccess.getCachedHeaderBuffer(conn)); Is this another round of testing with 2 headers needed? The connection has already been tested once with 2, then with 300. ------------- PR Review: https://git.openjdk.org/jdk/pull/30931#pullrequestreview-4370436610 PR Review Comment: https://git.openjdk.org/jdk/pull/30931#discussion_r3309289615 PR Review Comment: https://git.openjdk.org/jdk/pull/30931#discussion_r3309296482
