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());