Repository navigation
fix(eventloop): spend at most 16 reads on a conn per turn and queue it for another, so a conn whose inflow never pauses no longer starves the others on its worker (celeris#881) - #934
Draft
FumingPower3925 wants to merge 7 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Comment |
This was referenced Oct 3, 2026
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
FumingPower3925
force-pushed
the
fix/celeris-881-eventloop-read-budget
branch
from
October 3, 2026 12:00
540f85c to
2fd7fce
Compare
…ses the worker's eventfd or epoll fd number after shutdown closed it (celeris#862) The standalone driver loop's worker published its wakeup eventfd's number in a plain field. A Write that left bytes pending read it with no lock (wake, via enqueueFlush, after c.mu is released) while shutdown closed the eventfd and stored -1, so a Write running alongside Loop.Close was a data race, and could write 8 bytes to the number after the close, into whatever had taken it. The worker now holds the eventfd through internal/wakefd.WakeFD, the handle the engines adopted for the same defect (celeris#655, #666): Signal and Close share a lock, so a wake either completes before the close or writes nothing. RegisterConn had the same shape for the other descriptor a caller reaches: it read epollFD under w.mu, released w.mu, and issued EPOLL_CTL_ADD after. A shutdown in between closed the epoll fd, so the ADD went to the epoll instance that had taken the number, or to a closed number (RegisterConn then returned an epoll_ctl error for a conn whose onClose had already fired). The ADD is now issued under w.mu's read lock and c.mu, after a check that the conn has not been torn down, like every other epoll_ctl of a conn; shutdown marks every conn closed and closes the epoll fd under the write lock. A read lock, so a registration does not hold up the worker's lookups. A conn whose ADD fails is marked closed before it leaves the map. BenchmarkWritePending862 (the wake path) and BenchmarkRegisterChurn862 (a conn's round trip while other goroutines register and unregister on its worker) measure the cost; both run on the base too.
…d a RegisterConn racing UnregisterConn is tested (celeris#862) Review round 1 of #932. RegisterConn held w.mu's read lock across its EPOLL_CTL_ADD. A queued writer of w.mu (a forget, a registration, shutdown) then made the worker's lookups wait for the syscall, because a sync.RWMutex holds new readers back once a writer waits. The read lock is not needed: c is in the map, a conn leaves the map only once it is marked closed, and shutdown marks every conn in the map closed, under its c.mu, before it closes the epoll fd. So c.mu and the closed check alone keep the ADD off a closed or reused epoll fd number, the rule flushLocked already relies on. TestRegisterConnRacingUnregisterConnLeavesNoEpollEntry862 covers the other half of that check: an UnregisterConn of the same fd that runs in RegisterConn's window must leave fd out of the epoll set. A check of the epoll fd instead of c.closed passes both earlier tests and fails this one. The register-vs-Close test now fails, rather than skipping its main check, when no epoll instance takes the closed epoll fd's number, and the wake test's comment says that its coverage rests on -race.
…collected for, so a closed conn's stale event never reaches the conn that took its number (celeris#842) The standalone driver loop dispatched every event of an epoll_wait batch by the descriptor number it carried. An event collected for conn A, still in the batch when A was unregistered and closed and a new conn B registered A's number on the same worker, was applied to B: A's EPOLLRDHUP (A's server had hung up) tore B down, its onClose fired and its requests failed. Each registration now takes a per-worker generation (never 0) in RegisterConn, under w.mu, and every epoll_event of the registration carries it in Pad: the EPOLL_CTL_ADD, the two EPOLL_CTL_MODs in flushLocked and the WriteAndPoll* mask and re-arm (setEvents), all built by one helper so no MOD can drop it. The worker looks the conn up by number and drops the event unless the conn's generation is the event's. This is the shape #771 asks of the epoll engine's driver dispatch. The rest of the worker's number-keyed paths act on conns too: EPOLLOUT goes to the conn the event names, and the pending-flush list holds conns, not numbers, so a flush queued for a conn that has gone cannot reach a conn that took its number.
FumingPower3925
added a commit
that referenced
this pull request
Oct 3, 2026
…* call that holds the conn's recvMu (celeris#881) Review round 1 of #934. The read queue added a new way for the worker to wait on a conn's recvMu while a WriteAndPoll* call holds it. A conn whose turn stopped at readBudget is owed a turn in the next round. If its owner started a WriteAndPoll* call in between, serveReadQ waited on recvMu for the call's whole poll loop (about 50 ms for WriteAndPollMulti), and every other conn on the worker waited with it. The call's EPOLLIN mask does not prevent this, because a queued turn needs no event. A queued turn now only tries recvMu. When a call holds it, the worker drops the turn. The call reads the conn until EAGAIN, and the EPOLL_CTL_MOD that re-arms EPOLLIN at its end makes epoll report whatever is left as a new event, which dispatch serves. A turn that an event of the round's batch was merged into still waits for the lock. That event may be the re-arm's own report, collected before the call let go of recvMu, and dropping it would strand the bytes it reports. TestAQueuedTurnDoesNotWaitForAPollingCaller881 fails on the previous head. TestAQueuedTurnKeepsAnEventCollectedWhileACallerPolls881 fails if every busy turn is dropped. TestAWorkerWithAQueuedConnDoesNotWaitInEpollWait881 pins the zero epoll_wait timeout while a conn is queued. All three use nil-by-default test hooks. The recvMu comment now names the read turns.
FumingPower3925
force-pushed
the
fix/celeris-881-eventloop-read-budget
branch
from
October 3, 2026 13:44
2fd7fce to
53994be
Compare
…mment says what a wrap would take to misdeliver (celeris#842) Review round 2 of #933. worker.gen's comment said only that a generation is never 0. It now says why the 32-bit per-worker count can wrap without misdelivering: an event reaches the wrong conn only if the conn now registered on its number has the event's generation, which takes a multiple of 2^32-1 registrations on the worker between the two registrations while the event still waits to be dispatched. TestGenerationsWrapPastZero842 starts the worker's count just below the wrap and registers three conns: their generations are MaxUint32, 1 and 2, and each is served. No behaviour changes.
…t for another, so a conn whose inflow never pauses no longer starves the others on its worker (celeris#881) The standalone driver loop read a conn until EAGAIN for each event before it served the next one. A conn whose peer kept its socket non-empty kept the worker in that loop, and every other conn on the worker waited for it (BenchmarkReadWhileAnotherConnFlushes784: conn B's 1-byte round trip while conn A on the same worker carries an echo load). A read turn now stops after readBudget (16) reads, up to 256 KiB of the 16 KiB read buffer. A conn that used up its budget goes into a worker-local queue with the event's flags; the worker serves the queue after every batch and the pending flushes, and polls epoll without waiting while a conn is queued. Edge-triggered epoll does not report bytes already in the socket again, so the queue is what reads the rest, and the EPOLLRDHUP/EPOLLHUP/EPOLLERR teardown the event asked for runs once the socket is drained. Each backlogged conn gets one turn per round: a conn already queued when another of its events arrives is left to its queued turn, and a conn queued while the batch is dispatched waits for the next round, after the next epoll_wait, so the conns that became ready meanwhile are served first. The queue holds conns, not numbers, and a turn of a conn torn down since reads nothing (readOpen).
…* call that holds the conn's recvMu (celeris#881) Review round 1 of #934. The read queue added a new way for the worker to wait on a conn's recvMu while a WriteAndPoll* call holds it. A conn whose turn stopped at readBudget is owed a turn in the next round. If its owner started a WriteAndPoll* call in between, serveReadQ waited on recvMu for the call's whole poll loop (about 50 ms for WriteAndPollMulti), and every other conn on the worker waited with it. The call's EPOLLIN mask does not prevent this, because a queued turn needs no event. A queued turn now only tries recvMu. When a call holds it, the worker drops the turn. The call reads the conn until EAGAIN, and the EPOLL_CTL_MOD that re-arms EPOLLIN at its end makes epoll report whatever is left as a new event, which dispatch serves. A turn that an event of the round's batch was merged into still waits for the lock. That event may be the re-arm's own report, collected before the call let go of recvMu, and dropping it would strand the bytes it reports. TestAQueuedTurnDoesNotWaitForAPollingCaller881 fails on the previous head. TestAQueuedTurnKeepsAnEventCollectedWhileACallerPolls881 fails if every busy turn is dropped. TestAWorkerWithAQueuedConnDoesNotWaitInEpollWait881 pins the zero epoll_wait timeout while a conn is queued. All three use nil-by-default test hooks. The recvMu comment now names the read turns.
…d the queued-turn test clears its re-arm hook however it ends (celeris#881) Review round 2 of #934. No test had two conns backlogged at the same time, so the carry-over in serveReadQ (the tail of readQ: the conns queued during a round's batch) ran in no test. TestTwoBackloggedConnsGetEveryByte881 fills X1 with four turns of bytes, and X1's third read fills X2, so X2's first turn uses up its budget while X1 is still owed a turn. The test checks that on the worker, then needs every byte of both, in order, and each onClose(nil) after its last byte. Dropping the tail leaves X2 queued for a turn that never comes. TestAQueuedTurnKeepsAnEventCollectedWhileACallerPolls881 cleared testHookAfterRearm only on its success path. A cleanup now clears it once the call that runs it has returned. No behaviour changes.
FumingPower3925
force-pushed
the
fix/celeris-881-eventloop-read-budget
branch
from
October 3, 2026 15:27
53994be to
a21d0ff
Compare
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
1 participant
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.
Lane D1, PR 3 of 3, stacked on #933 (#842), which is stacked on #932 (#862). It targets
mainso that CI runs, and it carries both earlier commits. Review only this PR's three commits: d03bc69 (round 0, reviewed), 2ff7cb7 (review round 1, reviewed) and a21d0ff (review round 2: tests only, no behaviour change).Defect
The standalone driver event loop (
driver/internal/eventloop) read one conn until EAGAIN for each event, before it served the next event of the batch. That is what edge-triggered epoll needs. But a conn whose peer kept its socket non-empty never reached EAGAIN, so it held the worker, and every other conn on the worker waited until that conn's inflow paused.Mechanism (line numbers at 56a6c1e;
driver/internal/eventloopis byte-identical at 93bd88f, at df31adc, where the controls ran, and at 28383e8, current main)handleReadable(loop_linux.go:1125-1170) loopsreadOpenuntil EAGAIN, EOF or an error (:1146-1166), with no bound on the number of reads.run(:1088-1121) dispatches the next event only after it returns.:1093).Fix
readBudgetreads. That is 16 reads, up to 256 KiB of the worker's 16 KiB read buffer.readQ), with the event's flags. Edge-triggered epoll does not report bytes that are already in the socket again, so the queue is what reads the rest. While a conn is queued, the worker pollsepoll_waitwith timeout 0.EPOLLRDHUP/EPOLLHUP/EPOLLERRteardown runs once the socket is drained, as before; when the budget stops a turn first, the teardown runs on the turn that drains it.serveReadQserves the conns owed from earlier rounds. A conn that uses up its budget while the batch is dispatched waits for the next round, after the nextepoll_wait, so a conn that became ready during that turn is served first. A conn that is already queued when another of its events arrives is left to its queued turn.WriteAndPoll*call (review round 1).serveReadQonly tries the conn'srecvMu. When a call holds it, the worker drops the queued turn. The call reads the conn until EAGAIN, and theEPOLL_CTL_MODthat re-arms EPOLLIN at its end makes epoll report whatever is left (bytes, EOF, a hang-up) as a new event, whichdispatchserves. A turn that an event of this round's batch was merged into (readFresh) is not dropped: that event may be the re-arm's own report, collected before the call let go ofrecvMu, and no other event would come for the bytes it reports. That turn waits forrecvMu, asdispatchdoes on main for an event of a conn that is not queued (eventloop: the standalone driver loop's worker waits on a conn's recvMu while a WriteAndPoll* caller holds it, so every other conn on the worker waits for the caller's poll loop (about 50 ms for WriteAndPollMulti) #931).readOpen's closed check, fix(eventloop): never read a driver conn's descriptor number after UnregisterConn has returned (celeris#784) #843).testHookEpollWait(before eachepoll_wait, with the queue length and the timeout),testHookQueuedTurnBusy(a queued turn foundrecvMuheld) andtestHookAfterRearm(after aWriteAndPoll*call's re-arm MOD, with itsrecvMustill held).Deadlock check (RULE 10).
TryLocknever waits. The one blocking take that is left, for a turn with an event merged into it, is the samerecvMu.LockthatreadTurntakes for a dispatched event, with no other lock held.readTurnLockedruns withrecvMuheld, exactly asreadTurn's body did. The documented order (recvMu,w.mu,c.mu,c.rmu) is unchanged.Tests and controls
Three tests are added in
read_budget_881_linux_test.go:TestAConnWithEndlessInflowDoesNotStarveTheOthers881: A's peer writes as fast as A's socket takes bytes, and A'sonRecvis slow (a 100 µs sleep per chunk), so A's socket never empties. B, on the same worker, is sent one byte and must be served while A's inflow goes on (within 3 s). The test also counts A's reads between B's byte and B'sonRecvand allows at most two turns (32), so its bound does not rest on timing.TestAQueuedConnWaitsForTheNextRound881is deterministic. A's source is a pipe pre-filled with 1 MiB (four turns), and nothing more arrives. A's own thirdonRecvwrites B's byte, so B becomes ready during A's first turn. B must be served after exactly one turn of A's reads (16):epoll_waitgives 32 (mutant NOSNAP).TestBudgetedReadsDeliverEveryByteInOrder881: a pipe sized to 1 MiB is filled with a counter pattern before it is registered, so the first event finds 64 reads' worth (four turns) and nothing new will arrive. Every byte must arrive in order. Then the write end is closed, andonClose(nil)must fire after the last byte. This is the test that catches a budget without a working re-queue.Three more tests are added in
queued_turn_881_linux_test.go(review round 1). All three are deterministic: the worker is parked in P'sonRecvright after a turn of X (a pipe) that used up its budget and emptied X, so X is queued with nothing left in it (c881QueueXchecks the batch order andreadQueued).TestAQueuedTurnDoesNotWaitForAPollingCaller881(the review's MAJOR, in the shape of its probe). X's owner callsWriteAndPollMulti(X); its one-byte reply is written from inside the call's first read, after the mask, so the worker collects no event of X, and the call'sonRecvholds the call (and X'srecvMu) until B is served, for 5 s at most. The next round serves R's event, whoseonRecvsends B a byte, then X's queued turn. B must be served while the call still holds X'srecvMu. It fails on round 0's commit (theff-old881andnc-r1rows).TestAQueuedTurnKeepsAnEventCollectedWhileACallerPolls881. X's owner callsWriteAndPollMulti(X), which finds X empty and gives up. After its re-arm, withrecvMustill held (testHookAfterRearm), a bytezis written into X and P is released. The next round collects X's event forz, merges it into X's queued turn and findsrecvMuheld (testHookQueuedTurnBusy, which must report the merge); only then does the call let go.zmust reach X'sonRecv. Dropping every busy queued turn, as the review's experiment patch does (review-perf-api-security-r1/exp-trylock.patch), strandsz(mutant ALWAYSDROP).TestAWorkerWithAQueuedConnDoesNotWaitInEpollWait881(correctness MINOR). A pipe filled with 1 MiB (four turns) is registered; everyepoll_waitthe worker makes while a conn is queued must use timeout 0 (testHookEpollWait). With the idle 100 ms timeout, a backlogged conn would wait a full timeout per turn whenever nothing else arrives (mutant KEEPTIMEOUT, the review's).Review round 2 adds
TestTwoBackloggedConnsGetEveryByte881to the same file. It has two conns backlogged at once, soserveReadQmust carry a conn queued during a round's batch (the tail ofreadQ) over to the next round while it serves a conn owed from an earlier round. X1 is a pipe filled with 1 MiB (four turns) before it is registered, and X2 is a pipe registered empty. At X1's third read, X1'sonRecvfills X2 with 1 MiB, so X2's first event is in the batch of the round that owes X1 its second turn. X2's turn then uses up its budget while X1 is still queued. The test checks this on the worker, at X2's last read of that turn, and fails as a setup guard if it did not happen. Every byte of both conns must arrive in order, and each conn'sonClose(nil)must fire after its last byte once its write end is closed. The review's mutant TAILDROP, which drops that tail, leaves X2 queued for a turn that never comes. Round 2 also makesTestAQueuedTurnKeepsAnEventCollectedWhileACallerPolls881cleartestHookAfterRearmin a cleanup, once the call that runs the hook has returned, so that a failed check no longer leaves the hook installed.In round 0's tree the three round-1 hooks do not exist; the
ff-old881andnc-r1arms declare them in a test-only file, and they never run: the keeps-event test fails there on its wait for the re-arm hook, and the queued-wait test on its check that a conn was queued. Neither is a defect. The defect those two arms show is the caller test's. The round-1 tests' setup guards also fire under NOREQ and NOBUDGET, which never queue a conn, and under NOSERVE, whose queued turns never run. The "r1 setup guard" and "r2 setup guard" columns count those guard lines, one per failed guard: each is a test that could not drive its window, never a pass.Each arm runs 10 separate processes (
-race, linux/arm64, golang:1.27).fix-r2/logs/881-r2/ff-main.logfix-r2/logs/881-r2/ff-parent.logfix-r2/logs/881-r2/ff-old881.logfix-r2/logs/881-r2/fix.logfix-r2/logs/881-r2/nc.logfix-r2/logs/881-r2/nc-r1.logfix-r2/logs/881-r2/mut-NOREQ.logfix-r2/logs/881-r2/mut-NOBUDGET.logfix-r2/logs/881-r2/mut-NOSERVE.logfix-r2/logs/881-r2/mut-NOSNAP.logfix-r2/logs/881-r2/mut-NODROP.logfix-r2/logs/881-r2/mut-ALWAYSDROP.logfix-r2/logs/881-r2/mut-NOFRESH.logfix-r2/logs/881-r2/mut-KEEPTIMEOUT.logfix-r2/logs/881-r2/mut-TAILDROP.logfix-r2/logs/881-r2/parent-pkg.log)fix-r2/logs/881-r2/fix-pkg.log)fix-r2/logs/881-r2/fix-drivers.log)From the starvation test's own
C881 starvationlines:From the round test's
C881 roundslines, B became ready at A's read 3 and was served after A's read number: 16 in 10 of 10 processes on the head; 32 in 10 of 10 processes under NOSNAP; 64 in 10 of 10 processes on the base.NOBUDGET is "the budget check never fires", with
readBudgetleft at 16. Round 0 defined it asreadBudget = 1 << 30, andqueued_turn_881_linux_test.gosizes a buffer asreadBudgettimes 16 KiB, so under that definition every process was OOM-killed in the first test, before the starvation test ran (round 1'sfix-r1/logs/881-r1b/mut-NOBUDGET-oom.log: 10 processes, rc 137, no result line; absent, not a pass).Scripts:
bash evidence/lanes-20261003/D1/fix-r2/scripts/trees.sh df31adc dce5a3c aeeccf7 d03bc69 a21d0ff(the export trees), thenbash evidence/lanes-20261003/D1/fix-r2/scripts/controls.sh 881 r2. Table:python3 evidence/lanes-20261003/D1/fix-r2/scripts/table.py 881 evidence/lanes-20261003/D1/fix-r2/logs/881-r2.Main moved during round 2. #935 (#859) merged into main as 28383e8 while this round ran. It changes
driver/postgresand adds close-after-teardown tests to the three drivers; none of the commits since the base touchesdriver/internal/eventlooporinternal/wakefd. The stack is behind main, and it merges with it cleanly (git merge-tree --write-tree 28383e8 a21d0ff). The merged tree passes with-race: the eventloop package 39/0/0 (PASS/FAIL/SKIP) with 0 race reports, and./driver/... ./internal/wakefd/481/0/0 with 0 race reports, including #935's 7 #859 tests (evidence/lanes-20261003/D1/fix-r2/logs/merged-28383e8-a21d0ff/; scriptbash evidence/lanes-20261003/D1/fix-r2/scripts/merged-suite.sh 28383e8 a21d0ff).The round-1 review's probe at the head.
TestReviewQueuedTurnWaitsOnRecvMuOfAPollingCaller(the perf/API/security review's, uncommitted) runs as a correctness check, not a timing run: 10 separate-raceprocesses per arm in one laptop slot. B must be served within 20 ms of its byte while X's owner is inside aWriteAndPollMultiwhoseisDonenever fires.fix-r2/logs/probe-readq-r2/base.logfix-r2/logs/probe-readq-r2/old881.logfix-r2/logs/probe-readq-r2/h881.logScript:
bash evidence/lanes-20261003/D1/fix-r2/scripts/probe-readq-r2.sh r2 10 df31adc d03bc69 a21d0ff. Table:python3 evidence/lanes-20261003/D1/fix-r2/scripts/probe_table.py evidence/lanes-20261003/D1/fix-r2/logs/probe-readq-r2 '' base=base old881=r0 h881=head.The arms above run one after another, and the
basearm ran first, while the other slot ran this PR's#881mutants. The host was not quiet either, so itsWriteAndPollMultidurations are the host's, not main's. Round 1 had one slowh881process for the same reason. So the probe is also run with base and the head alternating, 40 processes each, so that both arms see the same load:fix-r2/logs/probe-interleave-r2/base.logfix-r2/logs/probe-interleave-r2/h881.logScript:
bash evidence/lanes-20261003/D1/fix-r2/scripts/probe-readq-interleave.sh r2 40. Table:python3 evidence/lanes-20261003/D1/fix-r2/scripts/probe_table.py evidence/lanes-20261003/D1/fix-r2/logs/probe-interleave-r2 '' base=base h881=head.Measurement:
BenchmarkReadWhileAnotherConnFlushes784, main and parent against headConn B's 1-byte round trip through the worker, while conn A on the same worker carries a pipelined TCP echo load. A's writer
Writeschunkbytes as fast as the 4 MiB cap allows (pausinggapbetweenWrites), and the server end echoes them back.chunk=0is the floor: no A. The sec/op, p50, p99 and p999 are B's; echoMB/s is A's throughput.Measured. Measured 2026-10-05, 01:32Z to 01:57Z, with
bash evidence/lanes-20261003/D1/fix-r1/scripts/bench.sh 10 10 df31adc dce5a3c aeeccf7 a21d0ff(the fix-r1 copy of the command, which differs fromfix-r2/scripts/bench.shonly in its output directory; dce5a3c, aeeccf7 and a21d0ff are the current heads of #932, #933 and #934). It ran under the laptop TIMING lock, in one linux/arm64 golang:1.27 container (--cpus 4): 10 rounds, one process per arm per round, the arm order rotated each round,-benchtime 1s, and an A/A arm (df31adc's test binary run a second time as its own arm). benchstat: median ± 95% CI, n=10 per arm,~= not significant at α=0.05. Host: no game client ran. 12 of the 25 one-minute samples during the run were NOT-QUIET by the script's rule, each because of one desktop process (Orca Helper, 49% to 58% of one of the 8 cores); the other 13 were quiet. The A/A floor: on the micro benchmarks the A/A arm differs from df31adc by 1.2% to 1.4% in 2 of 5 rows at p=0.023 and p=0.029, so a shift under about 1.5% there is not resolved.BenchmarkReadWhileAnotherConnFlushes784's sec/op CIs are ±13% to ±384% per arm, and its gapped rows are bimodal in every arm, the A/A arm included (round trips cluster near 20 µs and near 26 µs), so only large shifts there are resolved. Output:evidence/lanes-20261003/D1/fix-r1/bench/20261005T013145Z/(the per-arm*.txt,run.logwith the test binaries' sha256,quiet-during.log,benchstat/). Analysis:evidence/lanes-20261003/D1/fix-r3/scripts/analyse.sh(benchstat) andfix-r3/scripts/tables.py(these tables).The first four tables compare df31adc, the A/A arm, #933 (this PR's parent) and #934. Each delta is against df31adc. The next two compare #934 with #933 directly.
ReadWhileAnotherConnFlushes784/chunk=0/gap=0sReadWhileAnotherConnFlushes784/chunk=4096/gap=0sReadWhileAnotherConnFlushes784/chunk=65536/gap=0sReadWhileAnotherConnFlushes784/chunk=524288/gap=0sReadWhileAnotherConnFlushes784/chunk=262144/gap=1msReadWhileAnotherConnFlushes784/chunk=1048576/gap=5msReadWhileAnotherConnFlushes784/chunk=0/gap=0sReadWhileAnotherConnFlushes784/chunk=4096/gap=0sReadWhileAnotherConnFlushes784/chunk=65536/gap=0sReadWhileAnotherConnFlushes784/chunk=524288/gap=0sReadWhileAnotherConnFlushes784/chunk=262144/gap=1msReadWhileAnotherConnFlushes784/chunk=1048576/gap=5msReadWhileAnotherConnFlushes784/chunk=0/gap=0sReadWhileAnotherConnFlushes784/chunk=4096/gap=0sReadWhileAnotherConnFlushes784/chunk=65536/gap=0sReadWhileAnotherConnFlushes784/chunk=524288/gap=0sReadWhileAnotherConnFlushes784/chunk=262144/gap=1msReadWhileAnotherConnFlushes784/chunk=1048576/gap=5msReadWhileAnotherConnFlushes784/chunk=0/gap=0sReadWhileAnotherConnFlushes784/chunk=4096/gap=0sReadWhileAnotherConnFlushes784/chunk=65536/gap=0sReadWhileAnotherConnFlushes784/chunk=524288/gap=0sReadWhileAnotherConnFlushes784/chunk=262144/gap=1msReadWhileAnotherConnFlushes784/chunk=1048576/gap=5msReadWhileAnotherConnFlushes784/chunk=0/gap=0sReadWhileAnotherConnFlushes784/chunk=4096/gap=0sReadWhileAnotherConnFlushes784/chunk=65536/gap=0sReadWhileAnotherConnFlushes784/chunk=524288/gap=0sReadWhileAnotherConnFlushes784/chunk=262144/gap=1msReadWhileAnotherConnFlushes784/chunk=1048576/gap=5msReadWhileAnotherConnFlushes784/chunk=0/gap=0sReadWhileAnotherConnFlushes784/chunk=4096/gap=0sReadWhileAnotherConnFlushes784/chunk=65536/gap=0sReadWhileAnotherConnFlushes784/chunk=524288/gap=0sReadWhileAnotherConnFlushes784/chunk=262144/gap=1msReadWhileAnotherConnFlushes784/chunk=1048576/gap=5msHandleReadable784WriteAndPoll784/WriteAndPollWriteAndPoll784/WriteAndPollBusyWriteAndPoll784/WriteAndPollMultiWritePending862RegisterChurn862/churn=0RegisterChurn862/churn=1RegisterChurn862/churn=4RegisterChurn862/churn=0RegisterChurn862/churn=1RegisterChurn862/churn=4chunk=0do not shift.HandleReadable784, the threeWriteAndPoll784rows,WritePending862andRegisterChurn862sec/op are all ~ against df31adc (p=0.22 to 0.97).RegisterChurn862/churn=1B/op is +6.7% (918.5 to 980, p=0.009; churn=4 +6.7%, p=0.052) while allocs/op does not change. Those rows' CIs are ±11% and ±17%, and the A/A arm's equivalent rows are ~.Finding 1's shape (the review's probe, B's wait behind a
WriteAndPollMulticaller of a queued conn), as 10 processes per arm in the same run (python3 evidence/lanes-20261003/D1/fix-r2/scripts/probe_table.py evidence/lanes-20261003/D1/fix-r1/bench/20261005T013145Z probe- base=df31adc h862=#932 h842=#933 h881=#934):fix-r1/bench/20261005T013145Z/probe-base.logfix-r1/bench/20261005T013145Z/probe-h862.logfix-r1/bench/20261005T013145Z/probe-h842.logfix-r1/bench/20261005T013145Z/probe-h881.logAt the head, B's wait after its byte matches df31adc's (median 20.8 µs against 19.9 µs; ranges overlap). B is served in 10 of 10 processes in every arm.
Budget choice
Measured in the same run (the
h881b4andh881b64arms are the #881 head with onlyreadBudgetchanged; they ran only this benchmark). Each delta is against 16, the shipped value:ReadWhileAnotherConnFlushes784/chunk=0/gap=0sReadWhileAnotherConnFlushes784/chunk=4096/gap=0sReadWhileAnotherConnFlushes784/chunk=65536/gap=0sReadWhileAnotherConnFlushes784/chunk=524288/gap=0sReadWhileAnotherConnFlushes784/chunk=262144/gap=1msReadWhileAnotherConnFlushes784/chunk=1048576/gap=5msReadWhileAnotherConnFlushes784/chunk=0/gap=0sReadWhileAnotherConnFlushes784/chunk=4096/gap=0sReadWhileAnotherConnFlushes784/chunk=65536/gap=0sReadWhileAnotherConnFlushes784/chunk=524288/gap=0sReadWhileAnotherConnFlushes784/chunk=262144/gap=1msReadWhileAnotherConnFlushes784/chunk=1048576/gap=5msReadWhileAnotherConnFlushes784/chunk=0/gap=0sReadWhileAnotherConnFlushes784/chunk=4096/gap=0sReadWhileAnotherConnFlushes784/chunk=65536/gap=0sReadWhileAnotherConnFlushes784/chunk=524288/gap=0sReadWhileAnotherConnFlushes784/chunk=262144/gap=1msReadWhileAnotherConnFlushes784/chunk=1048576/gap=5msReadWhileAnotherConnFlushes784/chunk=0/gap=0sReadWhileAnotherConnFlushes784/chunk=4096/gap=0sReadWhileAnotherConnFlushes784/chunk=65536/gap=0sReadWhileAnotherConnFlushes784/chunk=524288/gap=0sReadWhileAnotherConnFlushes784/chunk=262144/gap=1msReadWhileAnotherConnFlushes784/chunk=1048576/gap=5msThe budget sets the trade between B's latency and A's throughput. 4 gives B the lowest tail (p99 −25% to −51% against 16) but leaves A at 1358 and 1534 MB/s (±123% to ±231%). 64 gives A df31adc's throughput back (4888 and 5410 MB/s, against df31adc's 4222 and 5456) but B's p50 is 4× to 4.4× that at 16 (190 and 158 µs). Even so, 64 is still below df31adc on B's p50 (405 and 438 µs) and p99 (60.7 and 9.1 ms against 0.72 and 0.52 ms). 16 keeps B's p50 within 2× to 2.5× of 4's and A's throughput at about 60% of df31adc's at 512 KiB. This run does not pick a value. It measures the trade, and it does not change the code.
Cost on the uncontended paths
Measured with the run above (see the uncontended sec/op and B/op tables under Measurement): no row on these paths shifts in sec/op or allocs/op against df31adc or #933 at α=0.05. A turn that does not use up its budget pays one compare per read and one
len(readQ)per round. A queued conn costs oneepoll_wait(0)per round and no allocation: the queue swaps two backing arrays. Round 1 adds aTryLockper queued turn, one bool store per event merged into a queued turn, and three nil checks of test hooks: one perepoll_wait, one perWriteAndPoll*re-arm, and one on the busy path of a queued turn.Family audit
handleReadable's read loop (the worker)drainOne/flushLocked(the worker's write side)Writecaps that at 4 MiB (maxPendingBytes).Writecannot append during a flush, because both holdc.muWriteAndPoll*drainsrecvMufor an event of the conn: one it collected before the call masked EPOLLIN, or the report of the call's re-arm when the owner has started its next call (#931; bounded by the call's poll phases, up to about 50 ms forWriteAndPollMulti). Round 0 of this PR added a second way, which needs no event: a queued turn waited onrecvMutoo (theold881row of the probe table). Round 1 removes it. A queued turn with no event merged into it only tries the lock; one with an event merged into it waits asdispatchdoes on main for that event, so #931's wait is no larger than on mainloop_other.go(the non-Linux fallback; it builds on darwin, and the drivers do not build on windows at all: #937)driverRead(engine/epoll/driver.go:345-376)engine/epoll, which #443 moves, so it is not in this PR.Not in this PR
recvMuwhile aWriteAndPoll*caller holds it, for an event of that conn, and every other conn on the worker waits with it. With round 1, this PR's queue no longer adds a way to reach that wait (see the audit table), and eventloop: the standalone driver loop's worker waits on a conn's recvMu while a WriteAndPoll* caller holds it, so every other conn on the worker waits for the caller's poll loop (about 50 ms for WriteAndPollMulti) #931 is corrected by a comment. Fixing eventloop: the standalone driver loop's worker waits on a conn's recvMu while a WriteAndPoll* caller holds it, so every other conn on the worker waits for the caller's poll loop (about 50 ms for WriteAndPollMulti) #931 by queueing a dispatched event that findsrecvMuheld is a design change with its own cost question (the conn must wait without spinningepoll_wait(0), and its turn cannot be dropped, for the reason in Fix), so it is not folded in here.Review round 1
recvMu. Fixed as the review suggested, with one change: a queued turn that an event of the conn was merged into still waits, because dropping it can strand bytes (mutant ALWAYSDROP, the keeps-event test). The family-audit row and eventloop: the standalone driver loop's worker waits on a conn's recvMu while a WriteAndPoll* caller holds it, so every other conn on the worker waits for the caller's poll loop (about 50 ms for WriteAndPollMulti) #931 are corrected (eventloop: the standalone driver loop's worker waits on a conn's recvMu while a WriteAndPoll* caller holds it, so every other conn on the worker waits for the caller's poll loop (about 50 ms for WriteAndPollMulti) #931 by a comment).epoll_waittimeout while a conn is queued.TestAWorkerWithAQueuedConnDoesNotWaitInEpollWait881does, with the review's mutant KEEPTIMEOUT as its second control.recvMucomment now names the read turns (readTurn, queued turns) instead ofhandleReadable.Review round 2
The correctness review approved. The perf/API/security review requested changes for one MAJOR, the measurement, and asked for no code change.
q[owed:]inserveReadQ) ran in no test.TestTwoBackloggedConnsGetEveryByte881is added, with the review's mutant TAILDROP as its second control. It needs the queue, so the failing-first arms and the NC do not run it. It passes on round 0's commit, which already carried the tail (theff-old881andnc-r1rows): it guards the queue, and is not a failing-first test.TestAQueuedTurnKeepsAnEventCollectedWhileACallerPolls881clearedtestHookAfterRearmonly on its success path. It now clears the hook in a cleanup.loop_other.gorow said "darwin/windows", but./driver/...does not build on windows. Filed as drivers: redis, postgres, memcached and the nine middleware stores built on them do not compile for GOOS=windows, which the README offers (unix.Dup in the fallback loop, int descriptors in each conn.go) #937 (pre-existing since v1.4.0), and the row now says so.Every control above was re-run at a21d0ff.
Fixes #881