From d5d9ee250bfb829d60db387496dd939cd1cd08cf Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 06:12:11 +0000 Subject: [PATCH] Make the ParallelGuard depth thread-local `PARALLEL_DEPTH` was a global `AtomicUsize`, so any thread entering a guarded region made every `step_resolution` job in the pool bounce off `is_in_parallel` and re-queue itself, even though those jobs would have run on threads that were free to take long work. The inversion the guard protects against is narrower than that: it happens only when a thread blocked inside a guarded `par_iter_mut` steals a long job and so cannot resume its parent. Stolen jobs run on the thread that stole them, so a thread-local depth is exactly the condition to test. Other threads keep picking up long jobs while a guarded region is active. Replaces the atomic with a `thread_local!` `Cell`; the guard holds an `EnteredSpan` and is therefore `!Send`, so it always drops on the thread that incremented the counter. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01B8hUmfq86DwNurgutaiL8e --- ext/src/utils.rs | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/ext/src/utils.rs b/ext/src/utils.rs index ea1c046eea..f8802e24e1 100644 --- a/ext/src/utils.rs +++ b/ext/src/utils.rs @@ -595,13 +595,22 @@ pub use logging::{LogWriter, ext_tracing_subscriber, init_logging}; pub(crate) mod parallel { - use std::sync::atomic::{AtomicUsize, Ordering}; + use std::cell::Cell; - static PARALLEL_DEPTH: AtomicUsize = AtomicUsize::new(0); + thread_local! { + static PARALLEL_DEPTH: Cell = const { Cell::new(0) }; + } /// RAII guard that increments [`PARALLEL_DEPTH`] on creation and decrements on drop. Used to mark /// regions where `par_iter_mut` work is active, so that `step_resolution` jobs can detect priority /// inversion and retry. + /// + /// The depth is thread-local because the priority inversion we are guarding against only happens + /// when the guarded thread itself steals a long job: while it is blocked inside a `par_iter_mut`, + /// rayon may hand it another job from the pool, and if that job is a `step_resolution` the parent + /// cannot resume until the long job finishes. A stolen job always runs on the OS thread that + /// stole it, so checking this thread's depth is exactly the condition we need. Other threads are + /// free to pick up long jobs in the meantime. pub(crate) struct ParallelGuard { #[allow(dead_code)] span: tracing::span::EnteredSpan, @@ -609,8 +618,11 @@ pub(crate) mod parallel { impl ParallelGuard { pub(crate) fn new() -> Self { - // We use Release to synchronize with `is_in_parallel` - let counter_start = PARALLEL_DEPTH.fetch_add(1, Ordering::Release); + let counter_start = PARALLEL_DEPTH.with(|depth| { + let old = depth.get(); + depth.set(old + 1); + old + }); Self { span: tracing::info_span!( "parallel_guard", @@ -624,14 +636,17 @@ pub(crate) mod parallel { impl Drop for ParallelGuard { fn drop(&mut self) { - // We use Release to synchronize with `is_in_parallel` - let counter_end = PARALLEL_DEPTH.fetch_sub(1, Ordering::Release); - self.span.record("counter_end", counter_end - 1); + let counter_end = PARALLEL_DEPTH.with(|depth| { + let new = depth.get() - 1; + depth.set(new); + new + }); + self.span.record("counter_end", counter_end); } } pub(crate) fn is_in_parallel() -> bool { - PARALLEL_DEPTH.load(Ordering::Acquire) > 0 + PARALLEL_DEPTH.with(|depth| depth.get()) > 0 } }