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. Thanks for taking a look; I've added comments just before the `getConnections()` and `getCachedHeaderBuffer()` to make it clear that these functions are only used in tests. As I understand, if we get rid of these functions, we'd need to use reflection, but that would break the tight coupling between the source and the tests. I thought we could add the `senderlock` assertion inside the `getCachedHeaderBuffer()` but the test does not (and cannot) grab the lock prior to calling `getCachedHeaderBuffer()` because it's private to `Http2Connection`. I _think_ it's still safe to access the buffer form the test because the test is single threaded and because the test doesn't access the buffer until the send request is complete, but let me know if you think we can make this safer. Also added the early return after `cacheHeaderBuffer` is newly created. ------------- PR Comment: https://git.openjdk.org/jdk/pull/30931#issuecomment-4548455211
