On Fri, 24 Apr 2026 22:02:15 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 two additional commits since > the last revision: > > - 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 53: > 51: public class HeaderEncodingBufferReuseTest { > 52: public static void main(String[] args) throws Exception { > 53: Http2TestServer server = new Http2TestServer("localhost", false, > 0); Please use the `HttpTestServer.create(...)` test library methods to create these servers. They are equipped to use bind to the right (loopback) addresses. For example: server = HttpTestServer.create(HTTP_2, sslContext); You will find several existing tests under `test/jdk/java/net/httpclient/` directory for reference. test/jdk/java/net/httpclient/http2/HeaderEncodingBufferReuseTest.java line 56: > 54: server.addHandler(new OkHandler(), "/test"); > 55: server.start(); > 56: String uri = "http://localhost:"; + server.getAddress().getPort() > + "/test"; Please use the `jdk.test.lib.net.URIBuilder` test library to construct these URIs. For this too, you will find several existing examples under `test/jdk/java/net/httpclient` directory. test/jdk/java/net/httpclient/http2/HeaderEncodingBufferReuseTest.java line 64: > 62: > 63: HttpResponse<Void> warmup = send(client, uri, 2); > 64: Asserts.assertEquals(warmup.version(), > HttpClient.Version.HTTP_2); Is the warmup needed in this test? test/jdk/java/net/httpclient/http2/HeaderEncodingBufferReuseTest.java line 68: > 66: Object conn = getHttp2Connection(client); > 67: Asserts.assertEquals(send(client, uri, 2).statusCode(), 200); > 68: ByteBuffer cached = (ByteBuffer) getField(conn, > "cachedHeaderBuffer"); I think this will need some slight adjustment to the test to assert that the connection that was used for the request was indeed the same one. There are ways to verify it, but I think we can get to that part once the rest of the review comments are addressed. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30931#discussion_r3152200997 PR Review Comment: https://git.openjdk.org/jdk/pull/30931#discussion_r3152205493 PR Review Comment: https://git.openjdk.org/jdk/pull/30931#discussion_r3152210508 PR Review Comment: https://git.openjdk.org/jdk/pull/30931#discussion_r3152223222
