On Wed, 6 May 2026 00:07:33 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. Use 'https' scheme with `.loopback()`, similar to other tests.
>   
>   2. Drop reflection to access the connection and the cached header
>      buffer.  Instead add accessor methods to HttpClientImplAccess that
>      fetch these members and use them in the tests.

src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java line 
1618:

> 1616:     private ByteBuffer cachedHeaderBuffer;
> 1617: 
> 1618:     ByteBuffer getCachedHeaderBuffer() {

I think this new method shouldn't be introduced. As far as I can see, it only 
gets used in a test. The `cachedHeaderBuffer` must be accessed with the 
`sendLock` held, which is what the `getHeaderBuffer(int size)` method (rightly) 
asserts. Having this package private getter can introduce issues if other parts 
of the code start using it in future.

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

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

Reply via email to