diff --git a/core/dr-gpu/src/readback.rs b/core/dr-gpu/src/readback.rs index af40d17..892b704 100644 --- a/core/dr-gpu/src/readback.rs +++ b/core/dr-gpu/src/readback.rs @@ -15,7 +15,28 @@ use crate::{GpuContext, GpuError}; /// surfacing the error. Set far above any plausible completion — the copies /// this waits on are milliseconds — so it is reached only when something is /// wrong. -const POLL_LIMIT: u32 = 100_000; +/// How long a readback may take before it is called failed. +/// +/// **A deadline, not an iteration count, and the difference was a real bug.** +/// This was 100,000 non-blocking polls, which sounds generous and is not: a +/// `Poll` that finds nothing returns immediately, so the loop burned through +/// the whole budget in a few milliseconds. Small transfers — the histogram's +/// 4 KB, a viewport-sized frame — happened to complete inside it. A +/// full-resolution export did not: 5472×3648 is 80 MB, and every attempt +/// failed with "readback did not complete" while the copy was still perfectly +/// healthy. +/// +/// Generous, because the legitimate worst case is a large export on a slow +/// integrated GPU, and the only thing this bound exists to catch is a lost +/// device that will never deliver the callback at all. +const READBACK_DEADLINE: std::time::Duration = std::time::Duration::from_secs(30); + +/// How long to wait between polls once the first few have found nothing. +/// +/// Without it this is a busy spin that saturates a core for the duration of +/// the copy. A millisecond is far below the transfer times involved and keeps +/// the thread available to the scheduler. +const POLL_PAUSE: std::time::Duration = std::time::Duration::from_millis(1); /// Drive the device until a `map_async` callback lands. /// @@ -34,7 +55,9 @@ pub(crate) fn await_mapping( rx: &std::sync::mpsc::Receiver>, ) -> Result<(), GpuError> { let mut mapped = None; - for _ in 0..POLL_LIMIT { + let deadline = std::time::Instant::now() + READBACK_DEADLINE; + let mut spins = 0u32; + while std::time::Instant::now() < deadline { // A poll error is a lost device, which is exactly the case the bounded // spin exists to escape — returning here reports it immediately rather // than spinning out the full limit first. @@ -46,11 +69,26 @@ pub(crate) fn await_mapping( mapped = Some(r); break; } - Err(std::sync::mpsc::TryRecvError::Empty) => continue, + Err(std::sync::mpsc::TryRecvError::Empty) => { + // The first handful of polls run flat out, so a small + // transfer — the histogram's, most of all — still returns + // without ever sleeping. Only a copy that is genuinely going + // to take a while pays the pause. + spins += 1; + if spins > 64 { + std::thread::sleep(POLL_PAUSE); + } + continue; + } Err(e) => return Err(GpuError::Readback(e.to_string())), } } mapped - .ok_or_else(|| GpuError::Readback("readback did not complete".into()))? + .ok_or_else(|| { + GpuError::Readback(format!( + "readback did not complete within {}s", + READBACK_DEADLINE.as_secs() + )) + })? .map_err(|e| GpuError::Readback(e.to_string())) }