From b18bf9186a89fbd37cf42da753c7b32d20f0bfd6 Mon Sep 17 00:00:00 2001 From: nazar-gavrylyk Date: Tue, 10 Nov 2026 15:09:40 +0100 Subject: [PATCH] Preview: GPU admission's reasoning holds for lanes four threads wide Only a lane's drain thread waits for a GPU slot. A lane runs one drain at a time (it is idle, and started again, only once its drain found no job), and its other threads only help with that frame's parallel work, as Rayon helpers that never wait. The comments said lanes had one thread; the test now runs on a lane LANE_WIDTH wide and checks that its other threads decline while the drain thread waits. --- crates/ui-egui/src/frames.rs | 10 +++++++--- crates/ui-egui/src/frames/tests.rs | 17 ++++++++++++----- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/frames.rs b/crates/ui-egui/src/frames.rs index c6f48ce0b..2b2b3d4b7 100644 --- a/crates/ui-egui/src/frames.rs +++ b/crates/ui-egui/src/frames.rs @@ -101,8 +101,12 @@ impl GpuRenderSlots { /// A Rayon helper may be executing a nested frame while an admitted render waits for /// its work. Blocking that helper on a permit would deadlock the render (#3028). fn for_render(&self, max: usize) -> Option> { - // Dedicated preview pools have one thread, protected from nested drains. They cannot - // be helpers of another admitted frame, so waiting preserves GPU preview throughput. + // Only a lane's drain thread waits: it renders one frame at a time, protected from + // nested drains, and a lane runs one drain at a time (it is idle again, and can be + // started anew, only once its drain found no job, so no frame of the lane is still + // running). The lane's other threads ([`LANE_WIDTH`]) only help with that frame's + // parallel work and are Rayon helpers here: they never wait. So no thread waits while + // an admitted frame waits for it, and waiting keeps the GPU preview's throughput. #[cfg(not(target_arch = "wasm32"))] if DRAINING_LANE.get() { return Some(self.acquire(max)); @@ -1720,7 +1724,7 @@ struct Worker { thread_local! { /// This pool thread is inside [`Worker::drain`] (rendering a frame). static DRAINING: std::cell::Cell = const { std::cell::Cell::new(false) }; - /// Only the single thread of a dedicated preview lane may wait for GPU admission. + /// Only a lane's drain thread may wait for GPU admission (see [`GpuRenderSlots::for_render`]). static DRAINING_LANE: std::cell::Cell = const { std::cell::Cell::new(false) }; } diff --git a/crates/ui-egui/src/frames/tests.rs b/crates/ui-egui/src/frames/tests.rs index 0a148b83c..baf6342e7 100644 --- a/crates/ui-egui/src/frames/tests.rs +++ b/crates/ui-egui/src/frames/tests.rs @@ -3325,18 +3325,19 @@ fn gpu_render_slots_never_block_rayon_helpers() { assert_eq!(*slots.busy.lock().unwrap(), 0); } -/// Dedicated preview lanes have one protected thread and can wait without creating the -/// shared-pool helper dependency cycle. They must keep rendering on a healthy GPU. +/// A lane's drain thread can wait without creating the shared-pool helper dependency cycle, +/// and must keep rendering on a healthy GPU; the lane's other threads ([`LANE_WIDTH`] wide) +/// help with its frame and never wait. #[cfg(not(target_arch = "wasm32"))] #[test] fn gpu_preview_lane_waits_for_admission_and_resets_its_marker() { let slots = GpuRenderSlots::new(); let first = slots.acquire(2); let second = slots.acquire(2); - let pool = rayon::ThreadPoolBuilder::new().num_threads(1).build().unwrap(); + let pool = rayon::ThreadPoolBuilder::new().num_threads(LANE_WIDTH).build().unwrap(); let (started, ready) = std::sync::mpsc::channel(); let (tx, rx) = std::sync::mpsc::channel(); - let (early, admitted) = std::thread::scope(|scope| { + let (early, admitted, helper) = std::thread::scope(|scope| { scope.spawn(|| { pool.install(|| { let _drain = Draining::enter(Some(0)).unwrap(); @@ -3347,13 +3348,19 @@ fn gpu_preview_lane_waits_for_admission_and_resets_its_marker() { }); }); ready.recv_timeout(std::time::Duration::from_secs(2)).unwrap(); + // Another thread of the same lane, while its drain thread waits: it declines. + let (htx, hrx) = std::sync::mpsc::channel(); + let (pool, slots) = (&pool, &slots); + scope.spawn(move || pool.install(|| htx.send(slots.for_render(2).is_none()).unwrap())); + let helper = hrx.recv_timeout(std::time::Duration::from_secs(2)); let early = rx.recv_timeout(std::time::Duration::from_millis(100)); drop((first, second)); let admitted = early.or_else(|_| rx.recv_timeout(std::time::Duration::from_secs(2))); - (early, admitted) + (early, admitted, helper) }); assert!(early.is_err(), "busy dedicated lane must wait instead of falling back to CPU"); assert_eq!(admitted, Ok(true)); + assert_eq!(helper, Ok(true), "a lane's helper thread must not wait on GPU permits"); pool.install(|| { assert!(!DRAINING.get()); assert!(!DRAINING_LANE.get());