diff --git a/crates/onnx-runtime-ep-cpu/src/decode_spmd.rs b/crates/onnx-runtime-ep-cpu/src/decode_spmd.rs index 7e1863e01d..9918651169 100644 --- a/crates/onnx-runtime-ep-cpu/src/decode_spmd.rs +++ b/crates/onnx-runtime-ep-cpu/src/decode_spmd.rs @@ -154,10 +154,6 @@ pub const WORKER_PROFILE_ENV: &str = "ONNX_GENAI_CPU_DECODE_WORKER_PROFILE"; /// pure spinning and only genuinely idle gaps ramp into yielding then parking. const SPIN_LOOP_BUDGET: u32 = 1 << 12; -/// Spin iterations between wall-clock checks, so `Instant::now()` (a vDSO read, -/// ~20 ns) is amortised over the hot spin loop rather than read every iteration. -const CLOCK_CHECK_STRIDE: u32 = 1 << 6; - /// How long [`SpmdDecodePools::build_with_schedule`]'s readiness barrier waits /// for every spawned worker to announce itself before declaring the pool /// unbuildable. @@ -859,7 +855,28 @@ impl SharedState { std::hint::spin_loop(); } else { thread::yield_now(); - if spins.is_multiple_of(CLOCK_CHECK_STRIDE) && start.elapsed() >= blocktime { + // Check the clock on *every* yield, not on a stride -- the same + // correction #1825 made to the readiness barrier below, which + // missed this site. The clock is never read during the pure + // spin phase here, so a stride amortised nothing: its only + // effect was to multiply the granularity of the blocktime + // deadline by 64 yields. `SPIN_LOOP_BUDGET` (4096) is itself a + // multiple of that removed 64-iteration stride, so the yield + // phase began exactly on a stride boundary -- the deadline was + // evaluated once, on the first yield, and then not again for 64 + // more. A yield costs microseconds to milliseconds under + // contention, and contention is exactly when a worker holding a + // core past the window it was told to release at does the most + // damage. `Instant::now()` is a vDSO read against a yield that + // costs orders of magnitude more -- measured on this host, + // 32ns per read against 1214ns for an *uncontended* + // `yield_now`, i.e. 2.6%, and the fraction only shrinks as + // contention makes the yield slower. Checking every time is + // free exactly where it matters. + // With the stride gone this file has no clock-stride constant + // left: its only use was in the phase its own doc comment said + // it did not apply to. + if start.elapsed() >= blocktime { break; } } diff --git a/crates/onnx-runtime-ep-cpu/src/task_runtime/pool.rs b/crates/onnx-runtime-ep-cpu/src/task_runtime/pool.rs index 668d72c567..e63af6322f 100644 --- a/crates/onnx-runtime-ep-cpu/src/task_runtime/pool.rs +++ b/crates/onnx-runtime-ep-cpu/src/task_runtime/pool.rs @@ -98,8 +98,11 @@ const MAX_SPIN: Duration = Duration::from_micros(500); /// Pure `spin_loop` iterations before a spinning worker starts yielding. const SPIN_LOOP_BUDGET: u32 = 1 << 12; -/// Spin iterations between wall-clock reads, so `Instant::now()` (~20 ns) is -/// amortised rather than paid every iteration. +/// Spin iterations between wall-clock reads *during the pure-`spin_loop` phase*, +/// so `Instant::now()` (~20 ns) is amortised rather than paid every iteration. +/// It deliberately does not gate the yield phase: a yield costs microseconds to +/// milliseconds under contention, so a stride there would multiply the spin +/// window's granularity by 64 yields of an already-starved thread (#1825). const CLOCK_CHECK_STRIDE: u32 = 1 << 6; /// Dispatcher spins between `yield_now` calls while waiting for stragglers. @@ -395,11 +398,34 @@ impl Shared { spins = spins.wrapping_add(1); if spins < SPIN_LOOP_BUDGET { std::hint::spin_loop(); + // The stride belongs here and only here: a `spin_loop` + // iteration costs nanoseconds, so an unamortised + // `Instant::now()` would dominate the phase. And the check + // has to run here at all, because at the converged idle + // window (`MIN_SPIN`, 20us) the deadline expires *before* + // the spin phase ends: 4096 `spin_loop`s measure 128us on + // this host. + if spins.is_multiple_of(CLOCK_CHECK_STRIDE) && start.elapsed() >= spin_window { + break; + } } else { thread::yield_now(); - } - if spins.is_multiple_of(CLOCK_CHECK_STRIDE) && start.elapsed() >= spin_window { - break; + // ...and must not apply here. A yield costs microseconds to + // milliseconds under contention, so a stride of 64 + // multiplies the window's granularity by 64 yields of an + // already-starved thread -- see #1825, which made this + // correction in `decode_spmd`'s readiness barrier. It bites + // whenever the window outlasts the spin phase, which is the + // whole grown range: 4096 `spin_loop`s measure 128us on + // this host, well under `MAX_SPIN` (500us), so every window + // above the floor reaches the yield phase. `MAX_SPIN`'s + // contract is that a process which stops inferencing + // returns to ~0% CPU in under a millisecond; a + // stride-gated check cannot honour that under exactly the + // load that makes it matter. + if start.elapsed() >= spin_window { + break; + } } } if caught {