From 60c53bfdc5b4b54d8a5377b77dff51fee79d2481 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 12 Oct 2026 22:16:52 -0700 Subject: [PATCH 1/2] quest(m1): plan lock-wait CPU from switches and the bench relay port race Follow-ups from #5277: derive lock-wait CPU from the recorded switch intervals instead of schedstat, and make the bench harness prove its relay owns the port before reporting it ready. Co-Authored-By: Claude Opus 5.5 --- quest/m1/perf/README.md | 2 ++ quest/m1/perf/bench-relay-port.md | 36 +++++++++++++++++++++++++++++++ quest/m1/perf/switch-cpu.md | 36 +++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+) create mode 100644 quest/m1/perf/bench-relay-port.md create mode 100644 quest/m1/perf/switch-cpu.md diff --git a/quest/m1/perf/README.md b/quest/m1/perf/README.md index 91543972277..08695ae1f9a 100644 --- a/quest/m1/perf/README.md +++ b/quest/m1/perf/README.md @@ -67,6 +67,8 @@ sample becomes its `moq-time` clock; it keeps #3122's measurement bar. ## Required - [Relay profiling](/quest/m1/perf/lock-profile.md) - one recipe captures lock wait by stack (first), CPU stacks, and heap profiles, with no code in kio +- [Lock-wait CPU from switches](/quest/m1/perf/switch-cpu.md) - wait/CPU takes CPU from the same perf recording as the waits, dropping schedstat and its ~2% low bias +- [Bench relay port](/quest/m1/perf/bench-relay-port.md) - the bench harness proves its relay owns the port before reporting ready - [kio channel contention](/quest/m1/perf/kio-channel-contention.md) - workers spend less time blocked on kio channel-state mutexes (up to 215% of worker CPU on chat at 16 tokio workers) - [One enter per turn](/quest/m1/perf/uring-one-enter.md) - a parking turn pays one io_uring_enter, submits flush deferred completions, and SQEs per enter is a counter - [Group cost](/quest/m1/perf/group-cost.md) - count and cut the allocations and time spent relaying one small group to one viewer diff --git a/quest/m1/perf/bench-relay-port.md b/quest/m1/perf/bench-relay-port.md new file mode 100644 index 00000000000..487fe33b4fe --- /dev/null +++ b/quest/m1/perf/bench-relay-port.md @@ -0,0 +1,36 @@ +# [S] Bench harness knows its relay owns the port + +## Goal + +`bench/relay.sh` reports a relay as ready only once that relay holds the port +it was given. A relay that loses the port fails the run at startup with the +relay's log, never as a load generator that silently receives zero groups. + +## Plan + +`start_relay` picks a random port in 40000-59999, starts the relay, and treats +any TCP connect to that port as readiness. If another process already listens +there, the probe succeeds against it. The relay then exits with "Address +already in use", and the harness only notices when the load generator receives +nothing. This happened once on a busy host during #5277. + +A shell harness cannot pick a port and bind it atomically for the relay, so +the open choice is where ownership gets proven. Settle it in the PR: + +- Recommended: keep the random port, and after the probe succeeds confirm that + `$RELAY_PID` is alive and owns both the TCP and UDP listeners on it (for + example with `ss -lnp`). Otherwise retry with a new port. Bench-only, with no + relay change. +- Let the relay bind port 0 and report its bound addresses. The relay prints + neither address today, and QUIC and HTTP would get different ports, so this + adds relay behavior and docs for a bench-only problem. +- Probe a relay-specific endpoint, such as its certificate fingerprint. This + proves the HTTP listener but not the QUIC socket. + +Cover it in `bench/relay.test.py` (already run by `just gh check`): hold the +chosen port open and expect the harness to retry or refuse. + +## Related + +- [Benchmark reporting](/bench/README.md) - the harness this fixes +- [Relay profiling](/quest/m1/perf/lock-profile.md) - where the race surfaced diff --git a/quest/m1/perf/switch-cpu.md b/quest/m1/perf/switch-cpu.md new file mode 100644 index 00000000000..be103536090 --- /dev/null +++ b/quest/m1/perf/switch-cpu.md @@ -0,0 +1,36 @@ +# [S] Lock-wait CPU from the recorded switches + +## Goal + +`just bench-lock` measures each worker's CPU from the same perf recording +that measures its waits, so wait/CPU shares one clock and one window, and +`bench/profile.py` stops reading `/proc` schedstat. + +## Plan + +Today `bench/profile.py record` reads schedstat before `perf record` starts and +after it exits, while perf records no switch-out samples for its first ~115 ms. +CPU therefore covers a longer window than the waits do, biasing wait/CPU about +2% low on the 5 s window. A worker that ran only during perf's start-up and then +slept would be refused ("ran but never switched out"), although the recording +is valid (CodeRabbit on #5277). + +Decided 2026-10-13 (maintainer AFK, recommended option taken): + +- Derive CPU from the `--switch-events` records already in `perf.data`: sum + each worker's on-CPU intervals, switch-in to the next switch-out, clipped to + the window the samples cover. Delete `schedstat` and the before/after reads. + - Rejected: gate perf through its `--control` FIFO and take schedstat at the + enable and disable points. It keeps two clocks and adds a control channel. +- A worker with no on-CPU time in the window is idle. A worker that ran in the + window but has no sampled switch-out still refuses. +- Bump `lock.json`'s `version`, since `cpu_ns` changes meaning. +- Extend `bench/profile.test.py` with a worker that runs only before the first + sample, and a check that CPU and waits share the window. Re-run + `just bench-lock fanout-heavy 16` and record the before/after wait/CPU in the + PR. + +## Related + +- [Relay profiling](/quest/m1/perf/lock-profile.md) - the recipe this corrects +- [kio channel contention](/quest/m1/perf/kio-channel-contention.md) - reads these wait/CPU numbers From 437a4563eb19f2d40819670d1f53e239bb6099cf Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 12 Oct 2026 22:22:29 -0700 Subject: [PATCH 2/2] quest(m1): keep schedstat to classify switchless workers A worker with no switch records either slept or ran the whole window, so the recording alone cannot call it idle (Codex review). Co-Authored-By: Claude Opus 5.5 --- quest/m1/perf/README.md | 2 +- quest/m1/perf/switch-cpu.md | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/quest/m1/perf/README.md b/quest/m1/perf/README.md index 08695ae1f9a..15553521c1c 100644 --- a/quest/m1/perf/README.md +++ b/quest/m1/perf/README.md @@ -67,7 +67,7 @@ sample becomes its `moq-time` clock; it keeps #3122's measurement bar. ## Required - [Relay profiling](/quest/m1/perf/lock-profile.md) - one recipe captures lock wait by stack (first), CPU stacks, and heap profiles, with no code in kio -- [Lock-wait CPU from switches](/quest/m1/perf/switch-cpu.md) - wait/CPU takes CPU from the same perf recording as the waits, dropping schedstat and its ~2% low bias +- [Lock-wait CPU from switches](/quest/m1/perf/switch-cpu.md) - wait/CPU takes CPU from the same perf recording as the waits, removing schedstat's ~2% low bias - [Bench relay port](/quest/m1/perf/bench-relay-port.md) - the bench harness proves its relay owns the port before reporting ready - [kio channel contention](/quest/m1/perf/kio-channel-contention.md) - workers spend less time blocked on kio channel-state mutexes (up to 215% of worker CPU on chat at 16 tokio workers) - [One enter per turn](/quest/m1/perf/uring-one-enter.md) - a parking turn pays one io_uring_enter, submits flush deferred completions, and SQEs per enter is a counter diff --git a/quest/m1/perf/switch-cpu.md b/quest/m1/perf/switch-cpu.md index be103536090..e35b8e347ca 100644 --- a/quest/m1/perf/switch-cpu.md +++ b/quest/m1/perf/switch-cpu.md @@ -3,8 +3,8 @@ ## Goal `just bench-lock` measures each worker's CPU from the same perf recording -that measures its waits, so wait/CPU shares one clock and one window, and -`bench/profile.py` stops reading `/proc` schedstat. +that measures its waits, so wait/CPU shares one clock and one window. +`/proc` schedstat only tells an idle worker from one that never switched. ## Plan @@ -19,11 +19,16 @@ Decided 2026-10-13 (maintainer AFK, recommended option taken): - Derive CPU from the `--switch-events` records already in `perf.data`: sum each worker's on-CPU intervals, switch-in to the next switch-out, clipped to - the window the samples cover. Delete `schedstat` and the before/after reads. + the window the samples cover. A worker's first record gives its state at the + window start: a switch-out means it was running. Schedstat no longer supplies + CPU. - Rejected: gate perf through its `--control` FIFO and take schedstat at the enable and disable points. It keeps two clocks and adds a control channel. -- A worker with no on-CPU time in the window is idle. A worker that ran in the - window but has no sampled switch-out still refuses. +- A worker with no switch records in the window either slept or ran + throughout, and the recording alone can't tell which (Codex on #5404). Keep + the schedstat reads only to classify those workers: if its counter did not + advance, the worker is idle; otherwise refuse the capture as ambiguous. + A worker with switch records but no sampled switch-out still refuses. - Bump `lock.json`'s `version`, since `cpu_ns` changes meaning. - Extend `bench/profile.test.py` with a worker that runs only before the first sample, and a check that CPU and waits share the window. Re-run