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

@@ -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<GpuRenderSlot<'_>> {
-        // 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<bool> = 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<bool> = const { std::cell::Cell::new(false) };
 }
 

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