Bound a readback by time, so a full-resolution export can finish
Exporting a real 20 MP CR2 failed every time with "readback did not complete", while the copy itself was perfectly healthy. The bound was 100,000 non-blocking polls. That sounds generous and is not: a `Poll` that finds nothing returns immediately, so the loop spent its entire budget in a few milliseconds. Small transfers — the histogram's 4 KB, a viewport-sized frame — happened to land inside it. An 80 MB frame never could. It is a deadline now, thirty seconds, which is the only thing the bound was ever for: catching a lost device that will never deliver the callback. A one-millisecond pause after the first sixty-four spins stops the loop saturating a core for the length of the copy, while keeping a small transfer as immediate as it was. Found by developing /home/dtourolle/Downloads/_MG_8596.CR2 through the export example: 5472×3648 renders in 127 ms and writes all five formats. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -15,7 +15,28 @@ use crate::{GpuContext, GpuError};
|
|||||||
/// surfacing the error. Set far above any plausible completion — the copies
|
/// surfacing the error. Set far above any plausible completion — the copies
|
||||||
/// this waits on are milliseconds — so it is reached only when something is
|
/// this waits on are milliseconds — so it is reached only when something is
|
||||||
/// wrong.
|
/// 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.
|
/// Drive the device until a `map_async` callback lands.
|
||||||
///
|
///
|
||||||
@@ -34,7 +55,9 @@ pub(crate) fn await_mapping(
|
|||||||
rx: &std::sync::mpsc::Receiver<Result<(), wgpu::BufferAsyncError>>,
|
rx: &std::sync::mpsc::Receiver<Result<(), wgpu::BufferAsyncError>>,
|
||||||
) -> Result<(), GpuError> {
|
) -> Result<(), GpuError> {
|
||||||
let mut mapped = None;
|
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
|
// A poll error is a lost device, which is exactly the case the bounded
|
||||||
// spin exists to escape — returning here reports it immediately rather
|
// spin exists to escape — returning here reports it immediately rather
|
||||||
// than spinning out the full limit first.
|
// than spinning out the full limit first.
|
||||||
@@ -46,11 +69,26 @@ pub(crate) fn await_mapping(
|
|||||||
mapped = Some(r);
|
mapped = Some(r);
|
||||||
break;
|
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())),
|
Err(e) => return Err(GpuError::Readback(e.to_string())),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
mapped
|
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()))
|
.map_err(|e| GpuError::Readback(e.to_string()))
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user