From c1ee8aae7632fc35de39591500955874fe88cbcf Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Thu, 14 Dec 2023 19:40:15 +0100 Subject: [PATCH 1/2] Remove the abstractions in resource maps ResourceMaps had a rather convoluted system for erasing types that isn't needed anywhere except in one place in the triage_resource function. Everywhere else we are always dealing with specific types so using a member of the resource maps is simpler than going through an abstraction. More importantly there was a constraint that all contents of the resource maps implement the Resource trait which got in the way of some of the ongoing buffer snatching changes. This commit simplifies this by removing the abstraction. Each resource type has its hash map directly in ResourceMaps and it is easier to have different requirements and behaviors depending on the type of the each resource. --- wgpu-core/src/device/global.rs | 13 ++ wgpu-core/src/device/life.rs | 270 ++++++++++++++++--------------- wgpu-core/src/device/queue.rs | 23 ++- wgpu-core/src/device/resource.rs | 40 +++-- 4 files changed, 196 insertions(+), 150 deletions(-) diff --git a/wgpu-core/src/device/global.rs b/wgpu-core/src/device/global.rs index 6f21e8bc6e..ded8c41da8 100644 --- a/wgpu-core/src/device/global.rs +++ b/wgpu-core/src/device/global.rs @@ -521,6 +521,7 @@ impl Global { device .lock_life() .suspected_resources + .buffers .insert(buffer_id, buffer); } @@ -784,6 +785,7 @@ impl Global { device .lock_life() .suspected_resources + .textures .insert(texture_id, texture.clone()); } } @@ -861,6 +863,7 @@ impl Global { view.device .lock_life() .suspected_resources + .texture_views .insert(texture_view_id, view.clone()); if wait { @@ -931,6 +934,7 @@ impl Global { .device .lock_life() .suspected_resources + .samplers .insert(sampler_id, sampler.clone()); } } @@ -1017,6 +1021,7 @@ impl Global { .device .lock_life() .suspected_resources + .bind_group_layouts .insert(bind_group_layout_id, layout.clone()); } } @@ -1080,6 +1085,7 @@ impl Global { .device .lock_life() .suspected_resources + .pipeline_layouts .insert(pipeline_layout_id, layout.clone()); } } @@ -1154,6 +1160,7 @@ impl Global { .device .lock_life() .suspected_resources + .bind_groups .insert(bind_group_id, bind_group.clone()); } } @@ -1448,6 +1455,7 @@ impl Global { .device .lock_life() .suspected_resources + .render_bundles .insert(render_bundle_id, bundle.clone()); } } @@ -1517,6 +1525,7 @@ impl Global { device .lock_life() .suspected_resources + .query_sets .insert(query_set_id, query_set.clone()); } } @@ -1653,10 +1662,12 @@ impl Global { let mut life_lock = device.lock_life(); life_lock .suspected_resources + .render_pipelines .insert(render_pipeline_id, pipeline.clone()); life_lock .suspected_resources + .pipeline_layouts .insert(layout_id, pipeline.layout.clone()); } } @@ -1788,9 +1799,11 @@ impl Global { let mut life_lock = device.lock_life(); life_lock .suspected_resources + .compute_pipelines .insert(compute_pipeline_id, pipeline.clone()); life_lock .suspected_resources + .pipeline_layouts .insert(layout_id, pipeline.layout.clone()); } } diff --git a/wgpu-core/src/device/life.rs b/wgpu-core/src/device/life.rs index 0193d0987b..3b4e73fdc7 100644 --- a/wgpu-core/src/device/life.rs +++ b/wgpu-core/src/device/life.rs @@ -14,111 +14,81 @@ use crate::{ TextureViewId, }, pipeline::{ComputePipeline, RenderPipeline}, - resource::{ - self, Buffer, QuerySet, Resource, ResourceType, Sampler, StagingBuffer, Texture, - TextureView, - }, + resource::{self, Buffer, QuerySet, Resource, Sampler, StagingBuffer, Texture, TextureView}, track::{ResourceTracker, Tracker}, FastHashMap, SubmissionIndex, }; use smallvec::SmallVec; use parking_lot::Mutex; +use std::sync::Arc; use thiserror::Error; -use wgt::WasmNotSendSync; - -use std::{any::Any, sync::Arc}; - -pub(crate) trait ResourceMap: Any + WasmNotSendSync { - fn as_any(&self) -> &dyn Any; - fn as_any_mut(&mut self) -> &mut dyn Any; - fn clear_map(&mut self); - fn extend_map(&mut self, maps: &mut ResourceMaps); -} - -impl ResourceMap for FastHashMap> -where - Id: id::TypedId, - R: Resource, -{ - fn as_any(&self) -> &dyn Any { - self - } - fn as_any_mut(&mut self) -> &mut dyn Any { - self - } - fn clear_map(&mut self) { - self.clear() - } - fn extend_map(&mut self, r: &mut ResourceMaps) { - if let Some(other) = r.maps.get_mut(R::TYPE) { - if let Some(other) = other.as_any_mut().downcast_mut::() { - self.extend(other.drain()); - } - } - } -} /// A struct that keeps lists of resources that are no longer needed by the user. #[derive(Default)] -pub(crate) struct ResourceMaps { - pub(crate) maps: FastHashMap>, +pub(crate) struct ResourceMaps { + pub(crate) buffers: FastHashMap>>, + pub(crate) staging_buffers: FastHashMap>>, + pub(crate) textures: FastHashMap>>, + pub(crate) texture_views: FastHashMap>>, + pub(crate) samplers: FastHashMap>>, + pub(crate) bind_groups: FastHashMap>>, + pub(crate) bind_group_layouts: FastHashMap>>, + pub(crate) render_pipelines: FastHashMap>>, + pub(crate) compute_pipelines: FastHashMap>>, + pub(crate) pipeline_layouts: FastHashMap>>, + pub(crate) render_bundles: FastHashMap>>, + pub(crate) query_sets: FastHashMap>>, } -impl ResourceMaps { - fn add_type(&mut self) -> &mut Self - where - Id: id::TypedId, - R: Resource, - { - let map = FastHashMap::>::default(); - self.maps.insert(R::TYPE, Box::new(map)); - self - } - fn map_mut(&mut self) -> &mut FastHashMap> - where - Id: id::TypedId, - R: Resource, - { - let map = self - .maps - .entry(R::TYPE) - .or_insert_with(|| Box::>>::default()); - let any_map = map.as_mut().as_any_mut(); - let map = any_map.downcast_mut::>>().unwrap(); - map - } - pub(crate) fn new() -> Self { - let mut maps = Self::default(); - maps.add_type::>(); - maps.add_type::>(); - maps.add_type::>(); - maps.add_type::>(); - maps.add_type::>(); - maps.add_type::>(); - maps.add_type::>(); - maps.add_type::>(); - maps.add_type::>(); - maps.add_type::>(); - maps.add_type::>(); - maps.add_type::>(); - maps +impl ResourceMaps { + pub(crate) fn new() -> Self { + ResourceMaps { + buffers: FastHashMap::default(), + staging_buffers: FastHashMap::default(), + textures: FastHashMap::default(), + texture_views: FastHashMap::default(), + samplers: FastHashMap::default(), + bind_groups: FastHashMap::default(), + bind_group_layouts: FastHashMap::default(), + render_pipelines: FastHashMap::default(), + compute_pipelines: FastHashMap::default(), + pipeline_layouts: FastHashMap::default(), + render_bundles: FastHashMap::default(), + query_sets: FastHashMap::default(), + } } + pub(crate) fn clear(&mut self) { - self.maps.iter_mut().for_each(|(_t, map)| map.clear_map()); + self.buffers.clear(); + self.staging_buffers.clear(); + self.textures.clear(); + self.texture_views.clear(); + self.samplers.clear(); + self.bind_groups.clear(); + self.bind_group_layouts.clear(); + self.render_pipelines.clear(); + self.compute_pipelines.clear(); + self.pipeline_layouts.clear(); + self.render_bundles.clear(); + self.query_sets.clear(); } + pub(crate) fn extend(&mut self, mut other: Self) { - self.maps.iter_mut().for_each(|(_t, map)| { - map.extend_map(&mut other); - }); - } - pub(crate) fn insert(&mut self, id: Id, r: Arc) -> &mut Self - where - Id: id::TypedId, - R: Resource, - { - self.map_mut().insert(id, r); - self + self.buffers.extend(other.buffers.drain()); + self.staging_buffers.extend(other.staging_buffers.drain()); + self.textures.extend(other.textures.drain()); + self.texture_views.extend(other.texture_views.drain()); + self.samplers.extend(other.samplers.drain()); + self.bind_groups.extend(other.bind_groups.drain()); + self.bind_group_layouts + .extend(other.bind_group_layouts.drain()); + self.render_pipelines.extend(other.render_pipelines.drain()); + self.compute_pipelines + .extend(other.compute_pipelines.drain()); + self.pipeline_layouts.extend(other.pipeline_layouts.drain()); + self.render_bundles.extend(other.render_bundles.drain()); + self.query_sets.extend(other.query_sets.drain()); } } @@ -140,7 +110,7 @@ struct ActiveSubmission { /// This includes things like temporary resources and resources that are /// used by submitted commands but have been dropped by the user (meaning that /// this submission is their last reference.) - last_resources: ResourceMaps, + last_resources: ResourceMaps, /// Buffers to be mapped once this submission has completed. mapped: Vec>>, @@ -214,7 +184,7 @@ pub(crate) struct LifetimeTracker { /// Resources whose user handle has died (i.e. drop/destroy has been called) /// and will likely be ready for destruction soon. - pub suspected_resources: ResourceMaps, + pub suspected_resources: ResourceMaps, /// Resources used by queue submissions still in flight. One entry per /// submission, with older submissions appearing before younger. @@ -229,7 +199,7 @@ pub(crate) struct LifetimeTracker { /// These are freed by `LifeTracker::cleanup`, which is called from periodic /// maintenance functions like `Global::device_poll`, and when a device is /// destroyed. - free_resources: ResourceMaps, + free_resources: ResourceMaps, /// Buffers the user has asked us to map, and which are not used by any /// queue submission still in flight. @@ -253,9 +223,9 @@ impl LifetimeTracker { mapped: Vec::new(), future_suspected_buffers: Vec::new(), future_suspected_textures: Vec::new(), - suspected_resources: ResourceMaps::new::(), + suspected_resources: ResourceMaps::new(), active: Vec::new(), - free_resources: ResourceMaps::new::(), + free_resources: ResourceMaps::new(), ready_to_map: Vec::new(), work_done_closures: SmallVec::new(), device_lost_closure: None, @@ -274,17 +244,19 @@ impl LifetimeTracker { temp_resources: impl Iterator>, encoders: Vec>, ) { - let mut last_resources = ResourceMaps::new::(); + let mut last_resources = ResourceMaps::new(); for res in temp_resources { match res { TempResource::Buffer(raw) => { - last_resources.insert(raw.as_info().id(), raw); + last_resources.buffers.insert(raw.as_info().id(), raw); } TempResource::StagingBuffer(raw) => { - last_resources.insert(raw.as_info().id(), raw); + last_resources + .staging_buffers + .insert(raw.as_info().id(), raw); } TempResource::Texture(raw) => { - last_resources.insert(raw.as_info().id(), raw); + last_resources.textures.insert(raw.as_info().id(), raw); } } } @@ -300,10 +272,12 @@ impl LifetimeTracker { pub fn post_submit(&mut self) { for v in self.future_suspected_buffers.drain(..).take(1) { - self.suspected_resources.insert(v.as_info().id(), v); + self.suspected_resources.buffers.insert(v.as_info().id(), v); } for v in self.future_suspected_textures.drain(..).take(1) { - self.suspected_resources.insert(v.as_info().id(), v); + self.suspected_resources + .textures + .insert(v.as_info().id(), v); } } @@ -379,13 +353,13 @@ impl LifetimeTracker { .map_or(&mut self.free_resources, |a| &mut a.last_resources); match temp_resource { TempResource::Buffer(raw) => { - resources.insert(raw.as_info().id(), raw); + resources.buffers.insert(raw.as_info().id(), raw); } TempResource::StagingBuffer(raw) => { - resources.insert(raw.as_info().id(), raw); + resources.staging_buffers.insert(raw.as_info().id(), raw); } TempResource::Texture(raw) => { - resources.insert(raw.as_info().id(), raw); + resources.textures.insert(raw.as_info().id(), raw); } } } @@ -408,8 +382,9 @@ impl LifetimeTracker { fn triage_resources( resources_map: &mut FastHashMap>, active: &mut [ActiveSubmission], - free_resources: &mut ResourceMaps, + free_resources: &mut ResourceMaps, trackers: &mut impl ResourceTracker, + get_resource_map: impl Fn(&mut ResourceMaps) -> &mut FastHashMap>, mut on_remove: T, ) -> Vec> where @@ -429,7 +404,7 @@ impl LifetimeTracker { if is_removed { on_remove(&id, resource); removed_resources.push(resource.clone()); - non_referenced_resources.insert(id, resource.clone()); + get_resource_map(non_referenced_resources).insert(id, resource.clone()); } !is_removed }); @@ -442,12 +417,13 @@ impl LifetimeTracker { #[cfg(feature = "trace")] trace: &mut Option<&mut trace::Trace>, ) -> &mut Self { let mut trackers = trackers.lock(); - let resource_map = self.suspected_resources.map_mut(); + let resource_map = &mut self.suspected_resources.render_bundles; let mut removed_resources = Self::triage_resources( resource_map, self.active.as_mut_slice(), &mut self.free_resources, &mut trackers.bundles, + |maps| &mut maps.render_bundles, |_bundle_id, _bundle| { #[cfg(feature = "trace")] if let Some(ref mut t) = *trace { @@ -457,19 +433,27 @@ impl LifetimeTracker { ); removed_resources.drain(..).for_each(|bundle| { for v in bundle.used.buffers.write().drain_resources() { - self.suspected_resources.insert(v.as_info().id(), v); + self.suspected_resources.buffers.insert(v.as_info().id(), v); } for v in bundle.used.textures.write().drain_resources() { - self.suspected_resources.insert(v.as_info().id(), v); + self.suspected_resources + .textures + .insert(v.as_info().id(), v); } for v in bundle.used.bind_groups.write().drain_resources() { - self.suspected_resources.insert(v.as_info().id(), v); + self.suspected_resources + .bind_groups + .insert(v.as_info().id(), v); } for v in bundle.used.render_pipelines.write().drain_resources() { - self.suspected_resources.insert(v.as_info().id(), v); + self.suspected_resources + .render_pipelines + .insert(v.as_info().id(), v); } for v in bundle.used.query_sets.write().drain_resources() { - self.suspected_resources.insert(v.as_info().id(), v); + self.suspected_resources + .query_sets + .insert(v.as_info().id(), v); } }); self @@ -481,12 +465,13 @@ impl LifetimeTracker { #[cfg(feature = "trace")] trace: &mut Option<&mut trace::Trace>, ) -> &mut Self { let mut trackers = trackers.lock(); - let resource_map = self.suspected_resources.map_mut(); + let resource_map = &mut self.suspected_resources.bind_groups; let mut removed_resource = Self::triage_resources( resource_map, self.active.as_mut_slice(), &mut self.free_resources, &mut trackers.bind_groups, + |maps| &mut maps.bind_groups, |_bind_group_id, _bind_group| { #[cfg(feature = "trace")] if let Some(ref mut t) = *trace { @@ -496,16 +481,22 @@ impl LifetimeTracker { ); removed_resource.drain(..).for_each(|bind_group| { for v in bind_group.used.buffers.drain_resources() { - self.suspected_resources.insert(v.as_info().id(), v); + self.suspected_resources.buffers.insert(v.as_info().id(), v); } for v in bind_group.used.textures.drain_resources() { - self.suspected_resources.insert(v.as_info().id(), v); + self.suspected_resources + .textures + .insert(v.as_info().id(), v); } for v in bind_group.used.views.drain_resources() { - self.suspected_resources.insert(v.as_info().id(), v); + self.suspected_resources + .texture_views + .insert(v.as_info().id(), v); } for v in bind_group.used.samplers.drain_resources() { - self.suspected_resources.insert(v.as_info().id(), v); + self.suspected_resources + .samplers + .insert(v.as_info().id(), v); } //Releasing safely unused resources to decrement refcount bind_group.used_buffer_ranges.write().clear(); @@ -513,6 +504,7 @@ impl LifetimeTracker { bind_group.dynamic_binding_info.write().clear(); self.suspected_resources + .bind_group_layouts .insert(bind_group.layout.as_info().id(), bind_group.layout.clone()); }); self @@ -524,12 +516,13 @@ impl LifetimeTracker { #[cfg(feature = "trace")] trace: &mut Option<&mut trace::Trace>, ) -> &mut Self { let mut trackers = trackers.lock(); - let resource_map = self.suspected_resources.map_mut(); + let resource_map = &mut self.suspected_resources.texture_views; let mut removed_resources = Self::triage_resources( resource_map, self.active.as_mut_slice(), &mut self.free_resources, &mut trackers.views, + |maps| &mut maps.texture_views, |_texture_view_id, _texture_view| { #[cfg(feature = "trace")] if let Some(ref mut t) = *trace { @@ -541,6 +534,7 @@ impl LifetimeTracker { let mut lock = texture_view.parent.write(); if let Some(parent_texture) = lock.take() { self.suspected_resources + .textures .insert(parent_texture.as_info().id(), parent_texture); } }); @@ -553,12 +547,13 @@ impl LifetimeTracker { #[cfg(feature = "trace")] trace: &mut Option<&mut trace::Trace>, ) -> &mut Self { let mut trackers = trackers.lock(); - let resource_map = self.suspected_resources.map_mut(); + let resource_map = &mut self.suspected_resources.textures; Self::triage_resources( resource_map, self.active.as_mut_slice(), &mut self.free_resources, &mut trackers.textures, + |maps| &mut maps.textures, |_texture_id, _texture| { #[cfg(feature = "trace")] if let Some(ref mut t) = *trace { @@ -575,12 +570,13 @@ impl LifetimeTracker { #[cfg(feature = "trace")] trace: &mut Option<&mut trace::Trace>, ) -> &mut Self { let mut trackers = trackers.lock(); - let resource_map = self.suspected_resources.map_mut(); + let resource_map = &mut self.suspected_resources.samplers; Self::triage_resources( resource_map, self.active.as_mut_slice(), &mut self.free_resources, &mut trackers.samplers, + |maps| &mut maps.samplers, |_sampler_id, _sampler| { #[cfg(feature = "trace")] if let Some(ref mut t) = *trace { @@ -597,12 +593,13 @@ impl LifetimeTracker { #[cfg(feature = "trace")] trace: &mut Option<&mut trace::Trace>, ) -> &mut Self { let mut trackers = trackers.lock(); - let resource_map = self.suspected_resources.map_mut(); + let resource_map = &mut self.suspected_resources.buffers; let mut removed_resources = Self::triage_resources( resource_map, self.active.as_mut_slice(), &mut self.free_resources, &mut trackers.buffers, + |maps| &mut maps.buffers, |_buffer_id, _buffer| { #[cfg(feature = "trace")] if let Some(ref mut t) = *trace { @@ -616,6 +613,7 @@ impl LifetimeTracker { } = *buffer.map_state.lock() { self.free_resources + .buffers .insert(stage_buffer.as_info().id(), stage_buffer.clone()); } }); @@ -628,12 +626,13 @@ impl LifetimeTracker { #[cfg(feature = "trace")] trace: &mut Option<&mut trace::Trace>, ) -> &mut Self { let mut trackers = trackers.lock(); - let resource_map = self.suspected_resources.map_mut(); + let resource_map = &mut self.suspected_resources.compute_pipelines; let mut removed_resources = Self::triage_resources( resource_map, self.active.as_mut_slice(), &mut self.free_resources, &mut trackers.compute_pipelines, + |maps| &mut maps.compute_pipelines, |_compute_pipeline_id, _compute_pipeline| { #[cfg(feature = "trace")] if let Some(ref mut t) = *trace { @@ -642,7 +641,7 @@ impl LifetimeTracker { }, ); removed_resources.drain(..).for_each(|compute_pipeline| { - self.suspected_resources.insert( + self.suspected_resources.pipeline_layouts.insert( compute_pipeline.layout.as_info().id(), compute_pipeline.layout.clone(), ); @@ -656,12 +655,13 @@ impl LifetimeTracker { #[cfg(feature = "trace")] trace: &mut Option<&mut trace::Trace>, ) -> &mut Self { let mut trackers = trackers.lock(); - let resource_map = self.suspected_resources.map_mut(); + let resource_map = &mut self.suspected_resources.render_pipelines; let mut removed_resources = Self::triage_resources( resource_map, self.active.as_mut_slice(), &mut self.free_resources, &mut trackers.render_pipelines, + |maps| &mut maps.render_pipelines, |_render_pipeline_id, _render_pipeline| { #[cfg(feature = "trace")] if let Some(ref mut t) = *trace { @@ -670,7 +670,7 @@ impl LifetimeTracker { }, ); removed_resources.drain(..).for_each(|render_pipeline| { - self.suspected_resources.insert( + self.suspected_resources.pipeline_layouts.insert( render_pipeline.layout.as_info().id(), render_pipeline.layout.clone(), ); @@ -684,7 +684,7 @@ impl LifetimeTracker { ) -> &mut Self { let mut removed_resources = Vec::new(); self.suspected_resources - .map_mut::>() + .pipeline_layouts .retain(|_pipeline_layout_id, pipeline_layout| { #[cfg(feature = "trace")] if let Some(ref mut t) = *trace { @@ -696,6 +696,7 @@ impl LifetimeTracker { removed_resources.drain(..).for_each(|pipeline_layout| { for bgl in &pipeline_layout.bind_group_layouts { self.suspected_resources + .bind_group_layouts .insert(bgl.as_info().id(), bgl.clone()); } }); @@ -706,9 +707,8 @@ impl LifetimeTracker { &mut self, #[cfg(feature = "trace")] trace: &mut Option<&mut trace::Trace>, ) -> &mut Self { - self.suspected_resources - .map_mut::>() - .retain(|bind_group_layout_id, bind_group_layout| { + self.suspected_resources.bind_group_layouts.retain( + |bind_group_layout_id, bind_group_layout| { //Note: this has to happen after all the suspected pipelines are destroyed //Note: nothing else can bump the refcount since the guard is locked exclusively //Note: same BGL can appear multiple times in the list, but only the last @@ -718,20 +718,23 @@ impl LifetimeTracker { t.add(trace::Action::DestroyBindGroupLayout(*bind_group_layout_id)); } self.free_resources + .bind_group_layouts .insert(*bind_group_layout_id, bind_group_layout.clone()); false - }); + }, + ); self } fn triage_suspected_query_sets(&mut self, trackers: &Mutex>) -> &mut Self { let mut trackers = trackers.lock(); - let resource_map = self.suspected_resources.map_mut(); + let resource_map = &mut self.suspected_resources.query_sets; Self::triage_resources( resource_map, self.active.as_mut_slice(), &mut self.free_resources, &mut trackers.query_sets, + |maps| &mut maps.query_sets, |_query_set_id, _query_set| {}, ); self @@ -739,9 +742,10 @@ impl LifetimeTracker { fn triage_suspected_staging_buffers(&mut self) -> &mut Self { self.suspected_resources - .map_mut::>() + .staging_buffers .retain(|staging_buffer_id, staging_buffer| { self.free_resources + .staging_buffers .insert(*staging_buffer_id, staging_buffer.clone()); false }); @@ -899,7 +903,9 @@ impl LifetimeTracker { if is_removed { *buffer.map_state.lock() = resource::BufferMapState::Idle; log::trace!("Buffer ready to map {:?} is not tracked anymore", buffer_id); - self.free_resources.insert(buffer_id, buffer.clone()); + self.free_resources + .buffers + .insert(buffer_id, buffer.clone()); } else { let mapping = match std::mem::replace( &mut *buffer.map_state.lock(), diff --git a/wgpu-core/src/device/queue.rs b/wgpu-core/src/device/queue.rs index 2552369649..2a7e4a77db 100644 --- a/wgpu-core/src/device/queue.rs +++ b/wgpu-core/src/device/queue.rs @@ -1153,8 +1153,7 @@ impl Global { // 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(); + let mut suspected = temp_suspected.replace(ResourceMaps::new()).unwrap(); suspected.clear(); } @@ -1224,7 +1223,11 @@ impl Global { unsafe { device.raw().unmap_buffer(raw_buf) } .map_err(DeviceError::from)?; } - temp_suspected.as_mut().unwrap().insert(id, buffer.clone()); + temp_suspected + .as_mut() + .unwrap() + .buffers + .insert(id, buffer.clone()); } else { match *buffer.map_state.lock() { BufferMapState::Idle => (), @@ -1246,7 +1249,11 @@ impl Global { }; texture.info.use_at(submit_index); if texture.is_unique() { - temp_suspected.as_mut().unwrap().insert(id, texture.clone()); + temp_suspected + .as_mut() + .unwrap() + .textures + .insert(id, texture.clone()); } if should_extend { unsafe { @@ -1262,6 +1269,7 @@ impl Global { temp_suspected .as_mut() .unwrap() + .texture_views .insert(texture_view.as_info().id(), texture_view.clone()); } } @@ -1281,6 +1289,7 @@ impl Global { temp_suspected .as_mut() .unwrap() + .bind_groups .insert(bg.as_info().id(), bg.clone()); } } @@ -1291,7 +1300,7 @@ impl Global { { compute_pipeline.info.use_at(submit_index); if compute_pipeline.is_unique() { - temp_suspected.as_mut().unwrap().insert( + temp_suspected.as_mut().unwrap().compute_pipelines.insert( compute_pipeline.as_info().id(), compute_pipeline.clone(), ); @@ -1302,7 +1311,7 @@ impl Global { { render_pipeline.info.use_at(submit_index); if render_pipeline.is_unique() { - temp_suspected.as_mut().unwrap().insert( + temp_suspected.as_mut().unwrap().render_pipelines.insert( render_pipeline.as_info().id(), render_pipeline.clone(), ); @@ -1314,6 +1323,7 @@ impl Global { temp_suspected .as_mut() .unwrap() + .query_sets .insert(query_set.as_info().id(), query_set.clone()); } } @@ -1334,6 +1344,7 @@ impl Global { temp_suspected .as_mut() .unwrap() + .render_bundles .insert(bundle.as_info().id(), bundle.clone()); } } diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index 5bd92242f2..86130d62f4 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -119,7 +119,7 @@ pub struct Device { life_tracker: Mutex>, /// Temporary storage for resource management functions. Cleared at the end /// of every call (unless an error occurs). - pub(crate) temp_suspected: Mutex>, + pub(crate) temp_suspected: Mutex>>, pub(crate) alignments: hal::Alignments, pub(crate) limits: wgt::Limits, pub(crate) features: wgt::Features, @@ -260,7 +260,7 @@ impl Device { valid: AtomicBool::new(true), trackers: Mutex::new(Tracker::new()), life_tracker: Mutex::new(life::LifetimeTracker::new()), - temp_suspected: Mutex::new(Some(life::ResourceMaps::new::())), + temp_suspected: Mutex::new(Some(life::ResourceMaps::new())), #[cfg(feature = "trace")] trace: Mutex::new(trace_path.and_then(|path| match trace::Trace::new(path) { Ok(mut trace) => { @@ -324,7 +324,7 @@ impl Device { let temp_suspected = self .temp_suspected .lock() - .replace(ResourceMaps::new::()) + .replace(ResourceMaps::new()) .unwrap(); let mut life_tracker = self.lock_life(); @@ -404,7 +404,7 @@ impl Device { let mut temp_suspected = self .temp_suspected .lock() - .replace(ResourceMaps::new::()) + .replace(ResourceMaps::new()) .unwrap(); temp_suspected.clear(); // As the tracker is cleared/dropped, we need to consider all the resources @@ -412,42 +412,58 @@ impl Device { { for resource in trackers.buffers.used_resources() { if resource.is_unique() { - temp_suspected.insert(resource.as_info().id(), resource.clone()); + temp_suspected + .buffers + .insert(resource.as_info().id(), resource.clone()); } } for resource in trackers.textures.used_resources() { if resource.is_unique() { - temp_suspected.insert(resource.as_info().id(), resource.clone()); + temp_suspected + .textures + .insert(resource.as_info().id(), resource.clone()); } } for resource in trackers.views.used_resources() { if resource.is_unique() { - temp_suspected.insert(resource.as_info().id(), resource.clone()); + temp_suspected + .texture_views + .insert(resource.as_info().id(), resource.clone()); } } for resource in trackers.bind_groups.used_resources() { if resource.is_unique() { - temp_suspected.insert(resource.as_info().id(), resource.clone()); + temp_suspected + .bind_groups + .insert(resource.as_info().id(), resource.clone()); } } for resource in trackers.samplers.used_resources() { if resource.is_unique() { - temp_suspected.insert(resource.as_info().id(), resource.clone()); + temp_suspected + .samplers + .insert(resource.as_info().id(), resource.clone()); } } for resource in trackers.compute_pipelines.used_resources() { if resource.is_unique() { - temp_suspected.insert(resource.as_info().id(), resource.clone()); + temp_suspected + .compute_pipelines + .insert(resource.as_info().id(), resource.clone()); } } for resource in trackers.render_pipelines.used_resources() { if resource.is_unique() { - temp_suspected.insert(resource.as_info().id(), resource.clone()); + temp_suspected + .render_pipelines + .insert(resource.as_info().id(), resource.clone()); } } for resource in trackers.query_sets.used_resources() { if resource.is_unique() { - temp_suspected.insert(resource.as_info().id(), resource.clone()); + temp_suspected + .query_sets + .insert(resource.as_info().id(), resource.clone()); } } } From d4b367d604f3fcad6705b844988e65a0d4c03e54 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Sun, 17 Dec 2023 15:32:13 +0100 Subject: [PATCH 2/2] Make it harder to forget to clear or extend a resource map --- wgpu-core/src/device/life.rs | 102 ++++++++++++++++++++++------------- 1 file changed, 64 insertions(+), 38 deletions(-) diff --git a/wgpu-core/src/device/life.rs b/wgpu-core/src/device/life.rs index 3b4e73fdc7..1daba3a6b8 100644 --- a/wgpu-core/src/device/life.rs +++ b/wgpu-core/src/device/life.rs @@ -27,18 +27,18 @@ use thiserror::Error; /// A struct that keeps lists of resources that are no longer needed by the user. #[derive(Default)] pub(crate) struct ResourceMaps { - pub(crate) buffers: FastHashMap>>, - pub(crate) staging_buffers: FastHashMap>>, - pub(crate) textures: FastHashMap>>, - pub(crate) texture_views: FastHashMap>>, - pub(crate) samplers: FastHashMap>>, - pub(crate) bind_groups: FastHashMap>>, - pub(crate) bind_group_layouts: FastHashMap>>, - pub(crate) render_pipelines: FastHashMap>>, - pub(crate) compute_pipelines: FastHashMap>>, - pub(crate) pipeline_layouts: FastHashMap>>, - pub(crate) render_bundles: FastHashMap>>, - pub(crate) query_sets: FastHashMap>>, + pub buffers: FastHashMap>>, + pub staging_buffers: FastHashMap>>, + pub textures: FastHashMap>>, + pub texture_views: FastHashMap>>, + pub samplers: FastHashMap>>, + pub bind_groups: FastHashMap>>, + pub bind_group_layouts: FastHashMap>>, + pub render_pipelines: FastHashMap>>, + pub compute_pipelines: FastHashMap>>, + pub pipeline_layouts: FastHashMap>>, + pub render_bundles: FastHashMap>>, + pub query_sets: FastHashMap>>, } impl ResourceMaps { @@ -60,35 +60,61 @@ impl ResourceMaps { } pub(crate) fn clear(&mut self) { - self.buffers.clear(); - self.staging_buffers.clear(); - self.textures.clear(); - self.texture_views.clear(); - self.samplers.clear(); - self.bind_groups.clear(); - self.bind_group_layouts.clear(); - self.render_pipelines.clear(); - self.compute_pipelines.clear(); - self.pipeline_layouts.clear(); - self.render_bundles.clear(); - self.query_sets.clear(); + let ResourceMaps { + buffers, + staging_buffers, + textures, + texture_views, + samplers, + bind_groups, + bind_group_layouts, + render_pipelines, + compute_pipelines, + pipeline_layouts, + render_bundles, + query_sets, + } = self; + buffers.clear(); + staging_buffers.clear(); + textures.clear(); + texture_views.clear(); + samplers.clear(); + bind_groups.clear(); + bind_group_layouts.clear(); + render_pipelines.clear(); + compute_pipelines.clear(); + pipeline_layouts.clear(); + render_bundles.clear(); + query_sets.clear(); } pub(crate) fn extend(&mut self, mut other: Self) { - self.buffers.extend(other.buffers.drain()); - self.staging_buffers.extend(other.staging_buffers.drain()); - self.textures.extend(other.textures.drain()); - self.texture_views.extend(other.texture_views.drain()); - self.samplers.extend(other.samplers.drain()); - self.bind_groups.extend(other.bind_groups.drain()); - self.bind_group_layouts - .extend(other.bind_group_layouts.drain()); - self.render_pipelines.extend(other.render_pipelines.drain()); - self.compute_pipelines - .extend(other.compute_pipelines.drain()); - self.pipeline_layouts.extend(other.pipeline_layouts.drain()); - self.render_bundles.extend(other.render_bundles.drain()); - self.query_sets.extend(other.query_sets.drain()); + let ResourceMaps { + buffers, + staging_buffers, + textures, + texture_views, + samplers, + bind_groups, + bind_group_layouts, + render_pipelines, + compute_pipelines, + pipeline_layouts, + render_bundles, + query_sets, + } = self; + buffers.extend(other.buffers.drain()); + staging_buffers.extend(other.staging_buffers.drain()); + textures.extend(other.textures.drain()); + texture_views.extend(other.texture_views.drain()); + samplers.extend(other.samplers.drain()); + bind_groups.extend(other.bind_groups.drain()); + bind_group_layouts.extend(other.bind_group_layouts.drain()); + render_pipelines.extend(other.render_pipelines.drain()); + compute_pipelines.extend(other.compute_pipelines.drain()); + pipeline_layouts.extend(other.pipeline_layouts.drain()); + render_bundles.extend(other.render_bundles.drain()); + query_sets.extend(other.query_sets.drain()); } }