forked from 1b-agent/opencadstudio-upstream
fix: never cache a buffer the device rejected
After an out-of-memory the viewport stopped updating for the rest of the session — both Model and Layout frozen, the UI still responsive. The log shows why: `block-wire-geometry` prints *after* the errors it provoked. The failed uploads were cached. A rejected allocation still yields a `Buffer`. It is invalid, every later use reports again, and it is indistinguishable from a good one at the cache key. So the entry keeps hitting, nothing ever rebuilds, and the frame stays empty even once memory frees up. Sharing `block_geometry` across slots in the previous commit made that worse, not better: one poisoned entry now serves every viewport. The device's uncaptured-error counter was already there, private to the handler that throttles the log. It is now readable, and the three caches that can retain a bad upload — block definition geometry, the shared wire buffer cache, and the arena's content id — snapshot it across their build and decline to record what it condemns. The degraded frame still draws; the next frame tries again. Best effort by construction: wgpu surfaces some errors asynchronously, so a moved counter means something failed, while an unmoved one is not proof that nothing did. That asymmetry is the right way round here — believing a bad buffer costs a frozen viewport, rebuilding a good one costs a frame. `block-wire-geometry` gains `poisoned=` so a run says whether this happened rather than leaving it to be inferred. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
1 parent
6599b82776
commit
d9c998fa17
3 files changed
+62
-11
No files matched your search
@@ -4956,12 +4956,25 @@ impl MultiPipeline {
|
|||||||
/// `Pipeline::new`, which runs again for every pane `ensure_len` adds. If iced
|
/// `Pipeline::new`, which runs again for every pane `ensure_len` adds. If iced
|
||||||
/// ever rebuilds the device it builds a new `MultiPipeline` too, so the fresh
|
/// ever rebuilds the device it builds a new `MultiPipeline` too, so the fresh
|
||||||
/// device is covered (and the throttle restarts with it).
|
/// device is covered (and the throttle restarts with it).
|
||||||
|
static GPU_ERRORS_SEEN: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0);
|
||||||
|
|
||||||
|
/// How many uncaptured device errors have been reported since this device was
|
||||||
|
/// created.
|
||||||
|
///
|
||||||
|
/// Callers snapshot it around an allocation to find out whether what they just
|
||||||
|
/// built is usable. Best effort: wgpu surfaces some errors asynchronously, so a
|
||||||
|
/// move means "something failed", while no move is not proof that nothing did.
|
||||||
|
/// That is the right asymmetry here — the cost of believing a bad buffer is a
|
||||||
|
/// frozen viewport, the cost of rebuilding a good one is a frame.
|
||||||
|
pub(crate) fn gpu_errors_seen() -> usize {
|
||||||
|
GPU_ERRORS_SEEN.load(std::sync::atomic::Ordering::Relaxed)
|
||||||
|
}
|
||||||
|
|
||||||
fn install_gpu_error_handler(device: &wgpu::Device) {
|
fn install_gpu_error_handler(device: &wgpu::Device) {
|
||||||
use std::sync::atomic::{AtomicUsize, Ordering};
|
use std::sync::atomic::Ordering;
|
||||||
static SEEN: AtomicUsize = AtomicUsize::new(0);
|
GPU_ERRORS_SEEN.store(0, Ordering::Relaxed);
|
||||||
SEEN.store(0, Ordering::Relaxed);
|
|
||||||
device.on_uncaptured_error(std::sync::Arc::new(|e: wgpu::Error| {
|
device.on_uncaptured_error(std::sync::Arc::new(|e: wgpu::Error| {
|
||||||
let n = SEEN.fetch_add(1, Ordering::Relaxed) + 1;
|
let n = GPU_ERRORS_SEEN.fetch_add(1, Ordering::Relaxed) + 1;
|
||||||
if n.is_power_of_two() {
|
if n.is_power_of_two() {
|
||||||
eprintln!("[gpu] uncaptured wgpu error #{n} (frame degraded, session kept alive): {e}");
|
eprintln!("[gpu] uncaptured wgpu error #{n} (frame degraded, session kept alive): {e}");
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -778,6 +778,8 @@ impl BlockWireGpu {
|
|||||||
// path's saving is visible rather than assumed.
|
// path's saving is visible rather than assumed.
|
||||||
let mut uploaded_bytes = 0usize;
|
let mut uploaded_bytes = 0usize;
|
||||||
let mut uploaded_segments = 0usize;
|
let mut uploaded_segments = 0usize;
|
||||||
|
// Definitions whose upload the device rejected, so they were not cached.
|
||||||
|
let mut poisoned = 0usize;
|
||||||
for (mesh_edge, group) in groups {
|
for (mesh_edge, group) in groups {
|
||||||
let Some(&source) = group.first() else {
|
let Some(&source) = group.first() else {
|
||||||
continue;
|
continue;
|
||||||
@@ -803,6 +805,12 @@ impl BlockWireGpu {
|
|||||||
push_block_batches(&mut out, device, queue, &chunks, &instances, mesh_edge);
|
push_block_batches(&mut out, device, queue, &chunks, &instances, mesh_edge);
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
// A failed allocation still produces a `Buffer`; it is simply
|
||||||
|
// invalid, and every later use of it reports again. Cached, it
|
||||||
|
// freezes the viewport for the rest of the session because the key
|
||||||
|
// keeps hitting and nothing ever rebuilds. Watch the device's error
|
||||||
|
// count across the build and refuse to cache what it condemns.
|
||||||
|
let errors_before = super::gpu_errors_seen();
|
||||||
let (segments, mut constant) = emit_wire_native(source, 0, color, 0.0);
|
let (segments, mut constant) = emit_wire_native(source, 0, color, 0.0);
|
||||||
if segments.is_empty() {
|
if segments.is_empty() {
|
||||||
continue;
|
continue;
|
||||||
@@ -906,8 +914,17 @@ impl BlockWireGpu {
|
|||||||
* std::mem::size_of::<BlockWireVertex>()
|
* std::mem::size_of::<BlockWireVertex>()
|
||||||
* if mode.uses_storage() { 1 } else { 6 };
|
* if mode.uses_storage() { 1 } else { 6 };
|
||||||
let chunks = std::sync::Arc::new(chunks);
|
let chunks = std::sync::Arc::new(chunks);
|
||||||
if let Some(cache) = cache.as_deref_mut() {
|
let built_clean = super::gpu_errors_seen() == errors_before;
|
||||||
cache.insert(cache_key, std::sync::Arc::clone(&chunks));
|
if built_clean {
|
||||||
|
if let Some(cache) = cache.as_deref_mut() {
|
||||||
|
cache.insert(cache_key, std::sync::Arc::clone(&chunks));
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
// Draw this frame with what we have — a degraded frame beats a
|
||||||
|
// blank one — but leave the cache clean so the next frame tries
|
||||||
|
// again. `live_keys` still holds the key, so the retain below
|
||||||
|
// does not evict a good entry from an earlier frame.
|
||||||
|
poisoned += 1;
|
||||||
}
|
}
|
||||||
let instances = block_instances(&group, base, mesh_edge, depth_map);
|
let instances = block_instances(&group, base, mesh_edge, depth_map);
|
||||||
push_block_batches(&mut out, device, queue, &chunks, &instances, mesh_edge);
|
push_block_batches(&mut out, device, queue, &chunks, &instances, mesh_edge);
|
||||||
@@ -919,12 +936,13 @@ impl BlockWireGpu {
|
|||||||
if crate::perf::enabled() && uploaded_segments > 0 {
|
if crate::perf::enabled() && uploaded_segments > 0 {
|
||||||
crate::perf_record!(
|
crate::perf_record!(
|
||||||
"[perf] block-wire-geometry mode={} definitions={} segments={} bytes={} \
|
"[perf] block-wire-geometry mode={} definitions={} segments={} bytes={} \
|
||||||
bytes_if_packed={}",
|
bytes_if_packed={} poisoned={}",
|
||||||
if mode.uses_storage() { "storage" } else { "packed" },
|
if mode.uses_storage() { "storage" } else { "packed" },
|
||||||
live_keys.len(),
|
live_keys.len(),
|
||||||
uploaded_segments,
|
uploaded_segments,
|
||||||
uploaded_bytes,
|
uploaded_bytes,
|
||||||
uploaded_segments * std::mem::size_of::<BlockWireVertex>() * 6,
|
uploaded_segments * std::mem::size_of::<BlockWireVertex>() * 6,
|
||||||
|
poisoned,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
out
|
out
|
||||||
|
|||||||
@@ -714,6 +714,9 @@ impl shader::Primitive for Primitive {
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
};
|
};
|
||||||
|
// Snapshot before any arena allocation, so a rejected
|
||||||
|
// upload cannot claim this content id below.
|
||||||
|
let arena_errors_before = crate::scene::pipeline::gpu_errors_seen();
|
||||||
let reg_ok = base_ok
|
let reg_ok = base_ok
|
||||||
&& if let Some(arena) = inner.wire_arena.as_mut() {
|
&& if let Some(arena) = inner.wire_arena.as_mut() {
|
||||||
let patch = patch.unwrap();
|
let patch = patch.unwrap();
|
||||||
@@ -868,7 +871,15 @@ impl shader::Primitive for Primitive {
|
|||||||
inner.wire_handle_index =
|
inner.wire_handle_index =
|
||||||
wire_arena::build_handle_index(&vp_wires[..]);
|
wire_arena::build_handle_index(&vp_wires[..]);
|
||||||
}
|
}
|
||||||
inner.wire_arena_id = vp.wire_content_id;
|
// Only claim the content id when the device accepted
|
||||||
|
// the upload; otherwise the slot believes it holds this
|
||||||
|
// content and never rebuilds it.
|
||||||
|
inner.wire_arena_id =
|
||||||
|
if crate::scene::pipeline::gpu_errors_seen() == arena_errors_before {
|
||||||
|
vp.wire_content_id
|
||||||
|
} else {
|
||||||
|
u64::MAX
|
||||||
|
};
|
||||||
arena_served = true;
|
arena_served = true;
|
||||||
} else {
|
} else {
|
||||||
inner.wire_arena = None;
|
inner.wire_arena = None;
|
||||||
@@ -922,6 +933,8 @@ impl shader::Primitive for Primitive {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
let t_upload = _perf.then(iced::time::Instant::now);
|
let t_upload = _perf.then(iced::time::Instant::now);
|
||||||
|
let errors_before =
|
||||||
|
crate::scene::pipeline::gpu_errors_seen();
|
||||||
let entry =
|
let entry =
|
||||||
inner.build_wire_buffers(device, queue, &vp_wires[..], &draw_depths);
|
inner.build_wire_buffers(device, queue, &vp_wires[..], &draw_depths);
|
||||||
if let Some(start) = t_upload {
|
if let Some(start) = t_upload {
|
||||||
@@ -933,9 +946,16 @@ impl shader::Primitive for Primitive {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
pipeline
|
// Buffers the device rejected are still `Buffer`s.
|
||||||
.wire_buffer_cache
|
// Cached under a content id that keeps matching,
|
||||||
.insert(vp.wire_content_id, entry.clone());
|
// they render nothing for the rest of the session
|
||||||
|
// and nothing ever rebuilds them. Draw the degraded
|
||||||
|
// frame, but let the next one try again.
|
||||||
|
if crate::scene::pipeline::gpu_errors_seen() == errors_before {
|
||||||
|
pipeline
|
||||||
|
.wire_buffer_cache
|
||||||
|
.insert(vp.wire_content_id, entry.clone());
|
||||||
|
}
|
||||||
entry
|
entry
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|||||||
Reference in new issue
Block a user