Repository navigation
Don't retain a read buffer for idle selector-based sockets on the JVM - #3790
Draft
sideeffffect wants to merge 5 commits into
Draft
sideeffffect wants to merge 5 commits into
sideeffffect wants to merge 5 commits into
Conversation
On the JVM, every socket allocated its 8 KiB read buffer when it was created and kept it for its whole life, even while waiting for data. For servers with many idle keep-alive connections this is the largest single item per connection. Allocate the read buffer when a read happens and drop it before waiting for the channel to become readable. `SelectingSocket.readChunk` now does a single non-blocking read, and `BufferedReads` waits via the new `awaitReadable` hook when no data is available, after releasing the buffer unless it holds partially read data (`readN`). Busy sockets keep reusing their buffer; a socket that has to wait for data allocates a new buffer when it wakes up. Other `BufferedReads` implementations never read 0 bytes, so for them only the initial allocation becomes lazy. Measured with 2000 idle loopback connections (4000 sockets, each with a pending `read(8192)`), heap after full GC, JDK 21: - before: 4002 HeapByteBuffers, 36.4 MB of byte[], ~21.5 KB per socket - after: 4 HeapByteBuffers, 3.5 MB of byte[], ~13.0 KB per socket Throughput on loopback (best of 5, two interleaved runs each): - 1 GiB bulk transfer, reads of 8 KiB: 1631-1736 ms before, 1694-1726 ms after (no measurable difference) - 20,000 request/response round trips of 100 bytes: 383-385 ms before, 445-447 ms after (one 8 KiB allocation per wake-up on each side) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The previous commit dropped a socket's read buffer while it waited for data, but then allocated a new 8 KiB buffer every time the socket woke up, which slowed down request/response traffic. `SelectingSocket` now reads directly in to a reusable direct buffer of the current thread and copies only the bytes read to a right-sized chunk, within the same synchronous step, so the scratch buffer never escapes and is never shared between fibers. `readN` accumulates the copied chunks across waits. Sockets hold no read buffer at all, and a woken socket allocates nothing but its result. As the buffer is direct, the JDK reads in to it without its own per-thread temporary buffer and the extra copy from it. `SelectingSocket` no longer extends `Socket.BufferedReads`, which is restored to its previous form for the other socket implementations. Measured on loopback, JDK 21, main / previous commit / this commit: Idle sockets (2000 connections = 4000 sockets, each with a pending `read(8192)`, heap after full GC): - per socket: 21.5 KB / 13.0 KB / 13.0 KB - HeapByteBuffers: 4002 / 4 / 4 20,000 request/response round trips of 100 bytes (`read(8192)`): - allocated: 257 MiB / 580 MiB / 248 MiB - best of 10, quietest run: 368 ms / 393 ms / 364 ms (an earlier quiet run of main vs previous commit: 383 ms / 445 ms) 1 GiB bulk transfer, 64 KiB writes, 8 KiB reads: - allocated: 2575 MiB / 2570 MiB / 2537 MiB - best of 10: 1566 ms / 1585 ms / 1605 ms (within noise) Timings are from a shared machine with background load, so the allocation numbers are the more reliable comparison. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`SocketBenchmark` measures the socket read path over loopback TCP, with fs2 on both ends: - `pingPong`: each of `connections` connections does 100 round trips of a 100 B message (echo server), reading with `read(8192)` - `bulk`: one connection requests 8 MiB and reads it with `read(8192)` Results with `-wi 5 -i 10 -w 2s -r 2s -prof gc`, two separate runs of one fork each per variant (20 measurement iterations), runs interleaved main / previous commits, under a lock so that no other JMH run on the machine overlapped. Load average during the runs was 2.6-13.9, so throughput is noisy; allocation is not. JDK 21, -Xmx256m. Variants: main = upstream main (f0d4244), lazy = 4e5d9e6 (drop buffer while waiting), scratch = 791d373 (per-thread scratch buffer). Throughput, ops/s (mean ± stdev of 20 iterations; per-run means in brackets): | benchmark | main | lazy | scratch | |----------------|-----------------------|------------------|------------------| | bulk | 200 ± 33 [170, 230] | 244 ± 6 [246, 242] | 262 ± 6 [260, 264] | | pingPong (1) | 284 ± 77 [212, 357] | 347 ± 14 [343, 352] | 363 ± 15 [356, 371] | | pingPong (16) | 84 ± 20 [72, 96] | 88 ± 5 [87, 88] | 95 ± 5 [91, 100] | The first main run was disturbed by load rising from 3.7 to 13.9; the second (quieter) main run is the better baseline. Allocation, gc.alloc.rate.norm (both ends of the connection): | benchmark | main | lazy | scratch | |----------------|---------------|---------------|---------------| | bulk, B/op | 10,580,563 | 10,492,241 | 10,293,485 | | pingPong (1), B/round trip | 22,142 | 37,997 | 21,774 | | pingPong (16), B/round trip | 22,706 | 39,138 | 22,235 | The lazy variant allocates an extra 8 KiB per wake-up on each end (+~16 KB per round trip); the scratch-buffer variant removes that and allocates slightly less than main, with no throughput regression. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Cap each thread's scratch buffer at 64 KiB, so that a single large read doesn't leave a large direct buffer on the thread for its lifetime. readN now copies the bytes straight from the scratch buffer in to the result array, which it allocates once the first bytes arrive, instead of concatenating chunks and compacting them. Add tests for reads larger than a single channel read, and for readN returning the remaining bytes at end-of-stream. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sideeffffect
changed the title
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.