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.

test/jdk/java/net/httpclient/http2/HeaderEncodingBufferReuseTest.java line 96:

> 94:                 .build()) {
> 95: 
> 96:             // Force a large cached header buffer by sending 300 headers.

The `bufferSize` of the cached buffer in the `Http2Connection` isn't decided by 
the number of headers. Instead, from what I see in 
`Http2Connection.encodeHeaders()` it's (almost fixed) size decided by the 
connection settings and a HttpClient configuration:

int bufferSize = Math.min(Math.max(getMaxSendFrameSize(), 1024), 
DEFAULT_FRAME_SIZE);


So this comment here is misleading and in fact it doesn't need creation of 300 
odd headers to test the change.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/30931#discussion_r3309321276

Reply via email to