nordic-dev.net/wgpu
nordic-dev.net/wgpu
2
0
Fork 0
mirror of https://github.com/gfx-rs/wgpu.git synced 2024-11-25 00:03:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

[core] Don't check for uniquely referenced resources at submission.

Browse Source 
Remove unreachable code from `Global::queue_submit` that checks
whether the resources used by the command buffer have a reference
count of one, and adds them to `Device::temp_suspected` if so.

When `queue_submit` is called, all the `Arc`s processed by this code
have a reference count of at least three, even when the user has
dropped the resource:

- `Device::trackers` holds strong references to all the device's
  resources.

- `CommandBufferMutable::trackers` holds strong references to all
  resources used by the command buffer.

- The `used_resources` methods of the various members of
  `CommandBufferMutable::trackers` all return iterators of owned
  `Arc`s.

Fortunately, since the `Global::device_drop_foo` methods all add the
`foo` being dropped to `Device::suspected_resources`, and
`LifetimeTracker::triage_suspected` does an adequate job of accounting
for the uninteresting `Arc`s and leaves the resources there until
they're actually dead, things do get cleaned up without the checks in
`Global::queue_submit`.

This allows `Device::temp_suspected` to be private to
`device::resource`, with a sole remaining use in `Device::untrack`.

Fixes #5647.
This commit is contained in:
Jim Blandy 2024-05-02 21:06:41 -07:00 committed by Erich Gubler
parent 565a0310e9
commit 5f464688e6
2 changed files with 19 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

94
wgpu-core/src/device/queue.rs
View File

@ -7,7 +7,7 @@ use crate::{
ClearError, CommandAllocator, CommandBuffer, CopySide, ImageCopyTexture, TransferError,
},
conv,
device::{life::ResourceMaps, DeviceError, WaitIdleError},
device::{DeviceError, WaitIdleError},
get_lowest_common_denom,
global::Global,
hal_api::HalApi,
@ -1183,11 +1183,6 @@ impl Global {
//TODO: if multiple command buffers are submitted, we can re-use the last
// native command buffer of the previous chain instead of always creating
// a temporary one, since the chains are not finished.
let mut temp_suspected = device.temp_suspected.lock();
{
let mut suspected = temp_suspected.replace(ResourceMaps::new()).unwrap();
suspected.clear();
}
// finish all the command buffers first
for &cmb_id in command_buffer_ids {
@ -1235,41 +1230,23 @@ impl Global {
// update submission IDs
for buffer in cmd_buf_trackers.buffers.used_resources() {
let tracker_index = buffer.info.tracker_index();
let raw_buf = match buffer.raw.get(&snatch_guard) {
Some(raw) => raw,
None => {
return Err(QueueSubmitError::DestroyedBuffer(
buffer.info.id(),
));
}
};
if buffer.raw.get(&snatch_guard).is_none() {
return Err(QueueSubmitError::DestroyedBuffer(
buffer.info.id(),
));
}
buffer.info.use_at(submit_index);
if buffer.is_unique() {
if let BufferMapState::Active { .. } = *buffer.map_state.lock()
{
log::warn!("Dropped buffer has a pending mapping.");
unsafe { device.raw().unmap_buffer(raw_buf) }
.map_err(DeviceError::from)?;
}
temp_suspected
.as_mut()
.unwrap()
.buffers
.insert(tracker_index, buffer.clone());
} else {
match *buffer.map_state.lock() {
BufferMapState::Idle => (),
_ => {
return Err(QueueSubmitError::BufferStillMapped(
buffer.info.id(),
))
}
match *buffer.map_state.lock() {
BufferMapState::Idle => (),
_ => {
return Err(QueueSubmitError::BufferStillMapped(
buffer.info.id(),
))
}
}
}
for texture in cmd_buf_trackers.textures.used_resources() {
let tracker_index = texture.info.tracker_index();
let should_extend = match texture.inner.get(&snatch_guard) {
None => {
return Err(QueueSubmitError::DestroyedTexture(
@ -1286,13 +1263,6 @@ impl Global {
}
};
texture.info.use_at(submit_index);
if texture.is_unique() {
temp_suspected
.as_mut()
.unwrap()
.textures
.insert(tracker_index, texture.clone());
}
if should_extend {
unsafe {
used_surface_textures
@ -1303,12 +1273,6 @@ impl Global {
}
for texture_view in cmd_buf_trackers.views.used_resources() {
texture_view.info.use_at(submit_index);
if texture_view.is_unique() {
temp_suspected.as_mut().unwrap().texture_views.insert(
texture_view.as_info().tracker_index(),
texture_view.clone(),
);
}
}
{
for bg in cmd_buf_trackers.bind_groups.used_resources() {
@ -1322,13 +1286,6 @@ impl Global {
for sampler in bg.used.samplers.used_resources() {
sampler.info.use_at(submit_index);
}
if bg.is_unique() {
temp_suspected
.as_mut()
.unwrap()
.bind_groups
.insert(bg.as_info().tracker_index(), bg.clone());
}
}
}
// assert!(cmd_buf_trackers.samplers.is_empty());
@ -1336,32 +1293,14 @@ impl Global {
cmd_buf_trackers.compute_pipelines.used_resources()
{
compute_pipeline.info.use_at(submit_index);
if compute_pipeline.is_unique() {
temp_suspected.as_mut().unwrap().compute_pipelines.insert(
compute_pipeline.as_info().tracker_index(),
compute_pipeline.clone(),
);
}
}
for render_pipeline in
cmd_buf_trackers.render_pipelines.used_resources()
{
render_pipeline.info.use_at(submit_index);
if render_pipeline.is_unique() {
temp_suspected.as_mut().unwrap().render_pipelines.insert(
render_pipeline.as_info().tracker_index(),
render_pipeline.clone(),
);
}
}
for query_set in cmd_buf_trackers.query_sets.used_resources() {
query_set.info.use_at(submit_index);
if query_set.is_unique() {
temp_suspected.as_mut().unwrap().query_sets.insert(
query_set.as_info().tracker_index(),
query_set.clone(),
);
}
}
for bundle in cmd_buf_trackers.bundles.used_resources() {
bundle.info.use_at(submit_index);
@ -1376,13 +1315,6 @@ impl Global {
for query_set in bundle.used.query_sets.read().used_resources() {
query_set.info.use_at(submit_index);
}
if bundle.is_unique() {
temp_suspected
.as_mut()
.unwrap()
.render_bundles
.insert(bundle.as_info().tracker_index(), bundle.clone());
}
}
}
let mut baked = cmdbuf.from_arc_into_baked();

25
wgpu-core/src/device/resource.rs
View File

@ -127,9 +127,6 @@ pub struct Device<A: HalApi> {
pub(crate) tracker_indices: TrackerIndexAllocators,
// Life tracker should be locked right after the device and before anything else.
life_tracker: Mutex<LifetimeTracker<A>>,
/// Temporary storage for resource management functions. Cleared at the end
/// of every call (unless an error occurs).
pub(crate) temp_suspected: Mutex<Option<ResourceMaps<A>>>,
/// Pool of bind group layouts, allowing deduplication.
pub(crate) bgl_pool: ResourcePool<bgl::EntryMap, BindGroupLayout<A>>,
pub(crate) alignments: hal::Alignments,
@ -142,6 +139,10 @@ pub struct Device<A: HalApi> {
#[cfg(feature = "trace")]
pub(crate) trace: Mutex<Option<trace::Trace>>,
pub(crate) usage_scopes: UsageScopePool<A>,
/// Temporary storage, cleared at the start of every call,
/// retained only to save allocations.
temp_suspected: Mutex<Option<ResourceMaps<A>>>,
}
pub(crate) enum DeferredDestroy<A: HalApi> {
@ -434,23 +435,9 @@ impl<A: HalApi> Device<A> {
let submission_closures =
life_tracker.triage_submissions(last_done_index, &self.command_allocator);
{
// Normally, `temp_suspected` exists only to save heap
// allocations: it's cleared at the start of the function
// call, and cleared by the end. But `Global::queue_submit` is
// fallible; if it exits early, it may leave some resources in
// `temp_suspected`.
let temp_suspected = self
.temp_suspected
.lock()
.replace(ResourceMaps::new())
.unwrap();
life_tracker.triage_suspected(&self.trackers);
life_tracker.suspected_resources.extend(temp_suspected);
life_tracker.triage_suspected(&self.trackers);
life_tracker.triage_mapped();
}
life_tracker.triage_mapped();
let mapping_closures =
life_tracker.handle_mapping(self.raw(), &self.trackers, &snatch_guard);