From 2f011f6ce4cb5339ec5adf3afafc2f75349d4e06 Mon Sep 17 00:00:00 2001 From: smarm-agent Date: Sun, 14 Jun 2026 20:01:02 +0000 Subject: [PATCH] fix(rfc-004): gate spinning on max_spinners>0, killing wake-nobody futex_wake at N=1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `spinning` was `spin_budget_cycles > 0` alone. At N=1 the pool-size cap is 0, so spinning was enabled while no scheduler could ever enlist: n_spinning pinned at 0, so the enqueue submit-rule guard (n_spinning==0) fired a wake-nobody futex_wake on every push. Now gated on `budget > 0 && max_spinners > 0`; a debug_assert guards the invariant against regression. No behavioural change (a 0-cap runtime never parks on the futex either way) — single_thread_spinning_is_inert and the AllDone liveness test (shutdown_releases_pinned_parked_siblings) still pass. switch_cost N=1 (1-core sandbox): p50 ~466ns -> ~186ns. The wake-nobody syscall was ~60% of the round-trip in wall-clock terms; the handoff's "~12%" was perf self-time, not latency share. ROADMAP per-switch section updated to fold in the N=1 spike correction (shim hypothesis is many-core only). Dropped the throwaway budget_probe bench (switch_cost reproduces every state via .spin_budget_cycles/ .max_spinners and with percentiles). --- ROADMAP.md | 33 ++++++++++++++++++++++++++++----- src/runtime.rs | 31 ++++++++++++++++++++++++++----- 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index f5be0b5..6d76e2d 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -120,11 +120,34 @@ Needs an RFC. #### Per-switch cost (context shims, epoch protocol) The shootout's residual: per-wake latency is 0.16–0.18 µs at N=1 and -0.8–1.2 µs at N=8+, dominated by the context-switch shims and the epoch -protocol, not the queue. On current evidence this is the larger constant — -"the whole game" alongside the v0.9 work — but there is no spec yet. Needs a -profiling spike (where do the cycles actually go per park/unpark round-trip) -and then an RFC before it can be scheduled. +0.8–1.2 µs at N=8+. The N=1 profiling spike is **done** (`docs/perswitch-profile-n1.md`): +it **revises the old shim-first framing**. At N=1 the context shims + TLS sp +accessors are ~1% of self-time, not the dominant cost — the shim hypothesis was +always a *many-core* one (its only evidence was the N=1→N=8 jump, blamed on TLS +access mode and cross-core coherency on the sp/epoch words), and nothing at N=1 +on one core can confirm or refute it. The N=1 round-trip is instead dominated by +the scheduler core (`schedule_loop` + run-queue, ~33%) plus a removable +wake-nobody `futex_wake`. + +That `futex_wake` is now **fixed**: it was an interaction between the RFC 004 +spin defaults and the N=1 pool-size cap of 0 — `spinning` was gated on +`budget > 0` alone, so at N=1 spinning was enabled while the cap forbade any +spinner from enlisting, pinning `n_spinning` at 0 and firing a wake-nobody +`futex_wake` on every enqueue. `spinning` is now gated on +`budget > 0 && max_spinners > 0` (a `debug_assert` guards the invariant). On the +1-core sandbox this cut N=1 p50 from ~466 ns to ~186 ns — the syscall was ~60% +of the round-trip in wall-clock terms (the handoff's "~12%" was perf *self-time*, +not latency share). + +Two separable targets remain: **(a)** the N=1 floor — `schedule_loop` + run-queue, +the irreducible scheduler core, no obvious cheap win left; **(b)** the N→8 slope — +shim / TLS / coherency, which needs the 5900X (HW-counter profiling; this sandbox +has no PMU). Still "the whole game" alongside v0.9. Next code chunk is a `remote` +mode for `benches/switch_cost.rs` (wake straddling two schedulers) — writable in +the sandbox, only measurable on the box. Do **not** optimise the shim until the +N=8 HW-counter data confirms it is the cost. Then an RFC; the +`spinning`-gating fix above may also warrant a small RFC since it touches the +RFC-004-settled futex path. #### send_after / cancel_timer Message-delivery timer on the existing min-heap (`timer.rs`): deliver a value to a diff --git a/src/runtime.rs b/src/runtime.rs index 5eca535..60bd651 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -540,10 +540,13 @@ pub(crate) struct RuntimeInner { /// wait immediately (EAGAIN) instead of sleeping — the lost-wakeup guard. /// One shared word; `futex_wake(1)` wakes exactly one parked scheduler. pub(crate) park_seq: AtomicU32, - /// RFC 004: `spin_budget_cycles > 0`. When false the feature is fully off — - /// the idle path is the historical `thread::sleep` park, `n_spinning` and - /// `park_seq` are never touched, and the submit rule is a no-op. This is - /// what makes `spin_budget_cycles = 0` recover today's behaviour exactly. + /// RFC 004: `spin_budget_cycles > 0 && max_spinners > 0`. When false the + /// feature is fully off — the idle path is the historical `thread::sleep` + /// park, `n_spinning` and `park_seq` are never touched, and the submit rule + /// is a no-op. Note the cap term: a budget with a 0 cap (e.g. the N=1 + /// pool-size gate) can enlist no spinner, so it is treated as off — this is + /// what makes `spin_budget_cycles = 0` AND the 0-cap case recover today's + /// behaviour exactly. pub(crate) spinning: bool, /// RFC 004: TSC cycles a scheduler searches before parking (`Config`). pub(crate) spin_budget_cycles: u64, @@ -593,6 +596,24 @@ impl RuntimeInner { let slots: Box<[Slot]> = (0..max_actors).map(|_| Slot::vacant()).collect(); // Low indices on top of the stack so early spawns get low pids. let free: Vec = (0..max_actors as u32).rev().collect(); + // RFC 004: the spin/futex machinery is only live when BOTH a budget is + // set AND the cap permits at least one spinner. A budget with a 0 cap + // (e.g. the N=1 pool-size gate) can never enlist a spinner, so + // `n_spinning` is pinned at 0 forever — which would make the enqueue + // submit-rule guard (`n_spinning == 0`) fire a wake-nobody futex_wake on + // every push. Gating on the cap too keeps that path inert exactly when + // it can do no work. Equivalent to budget=0 here, with no behavioural + // change (a 0-cap runtime never parks on the futex either way). + let spinning = spin_budget_cycles > 0 && max_spinners > 0; + // Guardrail: catch any future config path that re-enables spinning while + // leaving the cap at 0 — the pathology this gate exists to prevent. If + // we are spinning, at least one spinner must be enlistable. + debug_assert!( + !spinning || max_spinners > 0, + "spinning enabled (budget={spin_budget_cycles}) with max_spinners=0: \ + no scheduler can ever enlist, so every enqueue would fire a \ + wake-nobody futex_wake" + ); Arc::new(Self { run_queue: crate::run_queue::RunQueue::new(thread_count, max_actors), slots, @@ -601,7 +622,7 @@ impl RuntimeInner { queue_len: AtomicU64::new(0), n_spinning: AtomicU32::new(0), park_seq: AtomicU32::new(0), - spinning: spin_budget_cycles > 0, + spinning, spin_budget_cycles, max_spinners, timers: Mutex::new(Timers::new()),