Skip to content

socket, iroh: coalesce receives with UDP_GRO and stagger dial targets - #7

Merged
tmc merged 5 commits into
tmc:mainfrom
Mic92:relay-perf
Sep 15, 2026
Merged

tmc merged 5 commits into
tmc:mainfrom
Mic92:relay-perf

Conversation

@Mic92

@Mic92 Mic92 commented Aug 26, 2026 •

Copy link
Copy Markdown
Contributor

Rebased onto main and narrowed to the part that is still outstanding: the relay-path work from the original series has already merged, so the numbers that used to head this description no longer describe this branch. What is left is the direct path.

Three commits from @Mic92, plus one test:

  • socket: receive with UDP_GRO on Linux — one recvmsg returns a run of equally sized datagrams from the same peer instead of one datagram per syscall.
  • socket: pass whole receive batches to ReadFrom — the run is queued as a single strided recvBatch and handed out a datagram at a time from a cursor, rather than one batch per datagram.
  • iroh: stagger dial targets instead of trying them sequentially — Connect starts a handshake per target 250 ms apart and takes the first that completes, instead of spending a full handshake timeout per address before falling back to the relay. This fixes connect latency to peers advertising unreachable direct addresses.
  • socket: test that GRO records arrival addresses — see the rebase note below.

Measurements

The 32-byte regression raised against the first commit does not reproduce. Re-measured with the GRO commit isolated against its parent, -count=10, on three hosts, each with a base-vs-base null control run around the comparison to establish its own noise floor:

cell x86 (Xeon 8481C) arm (Neoverse) Cortex-X925, pinned
ConnMessageRate/size=32 −7.6% −0.8% −8.5%
ConnMessageRateWritev/size=32/batch=2 −7.2% −2.4% −8.4%
ConnMessageRateWritev/size=32/batch=8 −7.6% −2.1% −8.8%
ConnDatagramMessageRate/size=32 +3.2% ~ ~

(negative = faster; all cells shown are p < 0.01, ~ = no significant difference)

Small datagrams get faster on the stream path, on every host. The earlier −24% came from an unpinned big.LITTLE machine scheduling the two halves of the benchmark onto different core types between runs; pinned, it does not appear. The one real cost is ConnDatagramMessageRate at 32 bytes on x86, +3.2% — that host's null control was clean on that cell, so it is a genuine effect, though neither of the other two hosts reproduces it. The QUIC datagram path has no stream reassembly for the receive-side batching to pay back, which is the likely reason.

Rebase note

main now replies to each peer from the local address its datagrams arrived at (#24), which the ordinary receive loop maintains as it reads. serveGRO is a second receive loop, so it does not inherit that bookkeeping — merged as-is, GRO on a wildcard-bound Linux socket would silently undo #24 and replies would again leave from whatever address the route picks. The conflict is invisible to git rebase: the damage is in a loop neither side edits.

The resolution records the arrival address once per coalesced read — a coalesced run is by construction one peer's, so one address covers every datagram in it — and widens the control buffer to leave headroom beside the GRO control message. The final commit adds a test that binds wildcard and GRO together and asserts the arrival address survives a coalesced run; without the record it fails, as do four of #24's own tests.

Verified with -race on linux/arm64 and darwin/arm64; internal/socket cross-vets on linux/amd64, linux/arm64, darwin/arm64, windows/amd64, freebsd/amd64 and js/wasm.

@Mic92 Mic92 changed the title relay: cut CPU and allocations on the relay path (~3x throughput) relay: cut CPU and allocations on the relay path (~3x throughput), GRO on direct receive Aug 26, 2026
@tmc

tmc commented Aug 26, 2026

Copy link
Copy Markdown
Owner

Fantastic to see this! I'll do some of my own testing and let you know what I find.

@Mic92 Mic92 changed the title relay: cut CPU and allocations on the relay path (~3x throughput), GRO on direct receive Cut CPU and allocations on relay and direct paths (relay ~3x, direct +24%) Aug 26, 2026
@Mic92

Mic92 commented Aug 26, 2026 •

Copy link
Copy Markdown
Contributor Author

So that's my final numbers, my arm gets stiff from holding my laptop into the fan to make sure it doesn't get power throttled during measurements on this hot evening. Cheers.

@tmc

tmc commented Aug 27, 2026

Copy link
Copy Markdown
Owner

i did my own testing on a GCE c3-standard-8 box -- this almost all looks good, couple issues:

e212207: there's a race on the cursor. I know the comment says quic-go reads from a single
goroutine per Transport, and that's true of quic-go, but MagicConn is a net.PacketConn and
that interface says the opposite. Repro attached (3 senders / 8 readers, -race): 3/3 runs
give 18-22 races at transport.go:171/195 and panic: slice bounds out of range [64:0], each
inside a second. Quieter one in the same block — two goroutines can both hit the drain
condition and both call m.cur.release(), so one pooled buffer goes back twice. That one's
silent, which I think makes it the worse of the two.

Fine either way on the fix: lock the cursor, or keep it single-reader, say so on the type,
and drop the net.PacketConn claim. In fairness the same test finds 2 races in our own
recvAddrs map on the base commit — that one's ours, and I've fixed it with an RWMutex.

288e939: your CPU number result seems better than your commit message says — I got
-18.9%/-18.5% cpu-us/op across two runs, and 32B streams improved +7.9% both times.

What's holding it is what seems to be a regression at the 32B datagram size: -2.99% and
-3.55% across two runs, p<=0.009, with an identical-binary control run flat both times.
Happy to send the benchstat output.

847be27: small one. GetBuf(n) does *p = (*p)[:n] against a 65536-byte pool and nothing
enforces the doc's n <= MaxPacketSize, so a conforming max-size batch (65537) panics in
enqueue. Fix belongs in GetBuf — note enqueue immediately does *b = msg.AppendTo((*b)[:0]),
so the [:n] isn't doing anything except panicking. Can push that myself if easier.

Want to pull e212207 and 288e939 onto a separate branch for further iteration so the rest can go in?

tmc added a commit that referenced this pull request Aug 27, 2026
The first eleven commits of #7, by Jörg Thalheim, based on v0.1.0. They
reuse buffers on the relay send and receive paths and coalesce relay
datagrams into batch frames.
@tmc

tmc commented Aug 27, 2026

Copy link
Copy Markdown
Owner

I just merged 11 of the commits.

@tmc

tmc commented Aug 27, 2026

Copy link
Copy Markdown
Owner

I'm going to rebase this branch/PR so it just contains the unmerged commits (note: this means my shas in the previous comment are outdated).

tmc added a commit that referenced this pull request Aug 27, 2026
The last of #7's reviewed commits, by Jörg Thalheim. It needed to move
below the three that are still open in that PR.
@tmc

tmc commented Aug 27, 2026

Copy link
Copy Markdown
Owner

Merged most of this. main now has the first eleven plus the qng commit, hashes and authorship intact. I rebased and reordered your branch for you rather than making you do it — so resync with git fetch origin && git reset --hard origin/relay-perf, not git pull, which would merge the old history back in. Your old head was 8e6c71d if you'd rather redo it yourself.

Two of the three things I flagged are now fixed on our side, nothing needed from you:

  • GetBuf is bounded (7df95ed) — it allocates above MaxPacketSize instead of slicing a 64 KiB pooled buffer past its capacity. Your relayserver commit doesn't panic on a max-size batch anymore.
  • The recvAddrs race — the one I said was ours, not yours — is fixed with an RWMutex (146a5fa), so your branch won't trip it on the base commit.

What's left is the two real ones:

  • socket: receive with UDP_GRO — still the 32-byte datagram regression, -2.99% and -3.55% across two independent runs. If the datagram receive path can opt out of GRO, I think this lands.
  • socket: pass whole receive batches — the cursor race. Your call which way: lock it, or keep it single-reader, say so on the type, and drop the net.PacketConn claim.

Also, CI is red on something unrelated to any of that: iroh/endpoint.go imports internal/itls/tls twice in the dial-staggering commit, once as itls and once as tls (ST1019). One line.

That dial-staggering commit is new since I last looked, and it's a different topic from the relay work. Happy to review it — it'd just be easier as its own PR, so the two remaining commits here can land without waiting on it.

tmc added a commit that referenced this pull request Aug 28, 2026
The one commit from #7 that is ready to land, by Jörg Thalheim. It was
waiting on GetBuf holding any length (7df95ed), so a max-size batch no
longer panics in enqueue.

The two socket commits are still open on that PR, and the dial
staggering one is held back: it drops the early-data window from
ConnectEarly whenever a peer has more than one dial target.
@tmc

tmc commented Aug 28, 2026

Copy link
Copy Markdown
Owner

Rebased onto main and pushed to your relay-perf — resync with git fetch origin && git reset --hard origin/relay-perf, not git pull. Previous head was f77d67b.

relayserver: pool queued frame buffers is on main as 18b2a62, authorship intact.

The two socket commits are unchanged from my last comment: GRO needs the 32-byte regression addressed (an opt-out for the small-datagram path), and the batch commit needs the cursor race resolved either way. One data point on the latter — the single-reader premise does hold for our own use, qng's Transport.listen is the only ReadPacket caller, so documenting it and dropping the net.PacketConn claim is a fine answer.

On iroh: stagger dial targets: I fixed the ST1019 double import in place so the branch builds. The win is real and I measured both sides — three blackholed targets cost 15.0s on main and 5.5s on your branch. Two things before it lands:

It removes the application's early-data window from ConnectEarly. dialAny waits on <-qc.HandshakeComplete() before building the Connecting, so once there's more than one target Into0RTT returns ok=false — the transport still resumes (Used0RTT stays true), but the caller never gets to send early data. That's whenever a peer has direct addresses and a relay. The suite misses it because every 0-RTT test uses a single-target addr and takes the len(targets) == 1 fast path. Adding one blackholed decoy to TestConnectEarlyInto0RTTRoundTrip:

ConnectEarly returns Into0RTT
main 0s ok=true
your branch 252ms ok=false

That 252ms is a healthy path paying the stagger, and the caller can't prevent it. EndpointAddr.WithAddrs sorts and dedups the address set, so slot order comes from TransportAddr.Compare, not from insertion order or reachability. In that probe the blackholed 192.0.2.9 sorted ahead of the loopback server, so the working address didn't start until 250ms. Address sort order now sets connect latency for reachable peers.

Racing the dials rather than the handshakes would keep Connecting resolvable early and preserve 0-RTT. Separately, DialAttemptDelay is new exported API with a hardcoded 250ms — worth a deliberate decision.

Happy to push the multi-target 0-RTT test to main on its own; the coverage gap is worth closing regardless of where this commit ends up.

Last thing: now that thirteen of the sixteen commits have landed, the title and description describe mostly-merged work — the relay ~3x and direct +24% numbers are for changes that are already in. Could you retitle to cover what's actually left (something like "socket: receive with UDP_GRO and pass whole batches to ReadFrom") and trim the description to match? Happy to do it myself if you'd rather, I just didn't want to rewrite your benchmark claims without asking.

Mic92 and others added 5 commits September 14, 2026 17:55
IpTransport.Serve did one recvmsg per QUIC packet. With UDP_GRO the
kernel hands over runs of equally sized datagrams from one peer in a
single read (about 8 per call against a GSO sender on loopback). Read
into a pooled 64 KiB buffer, split by the GRO segment size from the
control message and return the buffer to the pool once ReadFrom has
copied out the last segment. Other platforms keep the per-packet loop.

AMD Ryzen AI 7 350, GOMAXPROCS=4, interleaved n=9, per 64 KiB op:
ConnStreamThroughput   905 -> 977 MB/s   (+8%)
                       183 -> 160 cpu-us  (-12%)
                       0 allocs/op unchanged
Both the GRO receive loop and the relay transport split a multi-segment
read into one recvBatch per datagram, so every QUIC packet still cost a
channel send, a select wakeup and an address lookup in ReadFrom. Queue
the whole read with its stride instead and let ReadFrom hand out one
segment per call from a cursor (quic-go reads from a single goroutine
per Transport), resolving the path address once per batch.

AMD Ryzen AI 7 350, GOMAXPROCS=4, interleaved n=9, per 64 KiB op:
ConnStreamThroughput        150 -> 140 cpu-us  (-6%), throughput ~1 GB/s unchanged
RelayConnStreamThroughput   297 -> 308 MB/s    (+4%)
                            424 -> 406 cpu-us  (-4%)
connectEarly walked the dial targets in turn, so a peer advertising N
unreachable direct addresses cost dialAttemptTimeout each before the
relay path was tried. Start a handshake per target dialAttemptDelay
apart and take the first that completes; a target already proven by the
path selector still short-circuits to the 0-RTT window.
The GRO receive loop had no test: it is compiled out off Linux and on Linux
nothing drove a coalesced read. Send a UDP_SEGMENT batch to a GRO-enabled
IpTransport over loopback and check Serve queues it as one strided recvBatch
that splits back into the datagrams sent.
The GRO receive loop is a second loop, so it does not inherit the
arrival-address bookkeeping the ordinary one does; a run coalesced by the
kernel must still record the local address it reached. Without the record
a reply to that peer leaves from whatever address the route picks, which
is the failure the arrival-address table exists to prevent.
@tmc

tmc commented Sep 15, 2026

Copy link
Copy Markdown
Owner

I'm going to rebase this and rework it so the remainder is mergable, FYI. Thanks for your contribution!

@tmc tmc changed the title Cut CPU and allocations on relay and direct paths (relay ~3x, direct +24%) socket, iroh: coalesce receives with UDP_GRO and stagger dial targets Sep 15, 2026
@tmc
tmc merged commit 85c4205 into tmc:main Sep 15, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants