From 3142e15907f3e29c1f72fe8b3f851f90add29d1a Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Thu, 4 Jul 2024 18:07:59 +0200 Subject: [PATCH] remove the triage suspected machinery --- wgpu-core/src/device/global.rs | 138 ++-------- wgpu-core/src/device/life.rs | 439 +------------------------------ wgpu-core/src/device/resource.rs | 86 +----- wgpu-core/src/lock/rank.rs | 7 - wgpu-core/src/resource.rs | 4 - wgpu-core/src/track/buffer.rs | 83 +----- wgpu-core/src/track/metadata.rs | 26 -- wgpu-core/src/track/mod.rs | 4 - wgpu-core/src/track/stateless.rs | 68 ----- wgpu-core/src/track/texture.rs | 74 +----- 10 files changed, 28 insertions(+), 901 deletions(-) diff --git a/wgpu-core/src/device/global.rs b/wgpu-core/src/device/global.rs index 2faefd914..d74e34d38 100644 --- a/wgpu-core/src/device/global.rs +++ b/wgpu-core/src/device/global.rs @@ -429,18 +429,9 @@ impl Global { buffer_id, ); - let last_submit_index = buffer.submission_index(); - - let device = buffer.device.clone(); - - device - .lock_life() - .suspected_resources - .buffers - .insert(buffer.tracker_index(), buffer); - if wait { - match device.wait_for_submit(last_submit_index) { + let last_submit_index = buffer.submission_index(); + match buffer.device.wait_for_submit(last_submit_index) { Ok(()) => (), Err(e) => log::error!("Failed to wait for buffer {:?}: {}", buffer_id, e), } @@ -613,18 +604,9 @@ impl Global { t.add(trace::Action::DestroyTexture(texture_id)); } - let last_submit_index = texture.submission_index(); - - let device = &texture.device; - - device - .lock_life() - .suspected_resources - .textures - .insert(texture.tracker_index(), texture.clone()); - if wait { - match device.wait_for_submit(last_submit_index) { + let last_submit_index = texture.submission_index(); + match texture.device.wait_for_submit(last_submit_index) { Ok(()) => (), Err(e) => log::error!("Failed to wait for texture {texture_id:?}: {e}"), } @@ -695,15 +677,8 @@ impl Global { t.add(trace::Action::DestroyTextureView(texture_view_id)); } - let last_submit_index = view.submission_index(); - - view.device - .lock_life() - .suspected_resources - .texture_views - .insert(view.tracker_index(), view.clone()); - if wait { + let last_submit_index = view.submission_index(); match view.device.wait_for_submit(last_submit_index) { Ok(()) => (), Err(e) => { @@ -758,18 +733,11 @@ impl Global { let hub = A::hub(self); - if let Some(sampler) = hub.samplers.unregister(sampler_id) { + if let Some(_sampler) = hub.samplers.unregister(sampler_id) { #[cfg(feature = "trace")] - if let Some(t) = sampler.device.trace.lock().as_mut() { + if let Some(t) = _sampler.device.trace.lock().as_mut() { t.add(trace::Action::DestroySampler(sampler_id)); } - - sampler - .device - .lock_life() - .suspected_resources - .samplers - .insert(sampler.tracker_index(), sampler.clone()); } } @@ -842,18 +810,11 @@ impl Global { let hub = A::hub(self); - if let Some(layout) = hub.bind_group_layouts.unregister(bind_group_layout_id) { + if let Some(_layout) = hub.bind_group_layouts.unregister(bind_group_layout_id) { #[cfg(feature = "trace")] - if let Some(t) = layout.device.trace.lock().as_mut() { + if let Some(t) = _layout.device.trace.lock().as_mut() { t.add(trace::Action::DestroyBindGroupLayout(bind_group_layout_id)); } - - layout - .device - .lock_life() - .suspected_resources - .bind_group_layouts - .insert(layout.tracker_index(), layout.clone()); } } @@ -926,18 +887,11 @@ impl Global { api_log!("PipelineLayout::drop {pipeline_layout_id:?}"); let hub = A::hub(self); - if let Some(layout) = hub.pipeline_layouts.unregister(pipeline_layout_id) { + if let Some(_layout) = hub.pipeline_layouts.unregister(pipeline_layout_id) { #[cfg(feature = "trace")] - if let Some(t) = layout.device.trace.lock().as_mut() { + if let Some(t) = _layout.device.trace.lock().as_mut() { t.add(trace::Action::DestroyPipelineLayout(pipeline_layout_id)); } - - layout - .device - .lock_life() - .suspected_resources - .pipeline_layouts - .insert(layout.tracker_index(), layout.clone()); } } @@ -1074,18 +1028,11 @@ impl Global { let hub = A::hub(self); - if let Some(bind_group) = hub.bind_groups.unregister(bind_group_id) { + if let Some(_bind_group) = hub.bind_groups.unregister(bind_group_id) { #[cfg(feature = "trace")] - if let Some(t) = bind_group.device.trace.lock().as_mut() { + if let Some(t) = _bind_group.device.trace.lock().as_mut() { t.add(trace::Action::DestroyBindGroup(bind_group_id)); } - - bind_group - .device - .lock_life() - .suspected_resources - .bind_groups - .insert(bind_group.tracker_index(), bind_group.clone()); } } @@ -1285,9 +1232,6 @@ impl Global { .unregister(command_encoder_id.into_command_buffer_id()) { cmd_buf.data.lock().as_mut().unwrap().encoder.discard(); - cmd_buf - .device - .untrack(&cmd_buf.data.lock().as_ref().unwrap().trackers); } } @@ -1371,18 +1315,11 @@ impl Global { let hub = A::hub(self); - if let Some(bundle) = hub.render_bundles.unregister(render_bundle_id) { + if let Some(_bundle) = hub.render_bundles.unregister(render_bundle_id) { #[cfg(feature = "trace")] - if let Some(t) = bundle.device.trace.lock().as_mut() { + if let Some(t) = _bundle.device.trace.lock().as_mut() { t.add(trace::Action::DestroyRenderBundle(render_bundle_id)); } - - bundle - .device - .lock_life() - .suspected_resources - .render_bundles - .insert(bundle.tracker_index(), bundle.clone()); } } @@ -1432,19 +1369,11 @@ impl Global { let hub = A::hub(self); - if let Some(query_set) = hub.query_sets.unregister(query_set_id) { - let device = &query_set.device; - + if let Some(_query_set) = hub.query_sets.unregister(query_set_id) { #[cfg(feature = "trace")] - if let Some(trace) = device.trace.lock().as_mut() { + if let Some(trace) = _query_set.device.trace.lock().as_mut() { trace.add(trace::Action::DestroyQuerySet(query_set_id)); } - - device - .lock_life() - .suspected_resources - .query_sets - .insert(query_set.tracker_index(), query_set.clone()); } } @@ -1675,24 +1604,11 @@ impl Global { let hub = A::hub(self); - if let Some(pipeline) = hub.render_pipelines.unregister(render_pipeline_id) { - let device = &pipeline.device; - + if let Some(_pipeline) = hub.render_pipelines.unregister(render_pipeline_id) { #[cfg(feature = "trace")] - if let Some(t) = pipeline.device.trace.lock().as_mut() { + if let Some(t) = _pipeline.device.trace.lock().as_mut() { t.add(trace::Action::DestroyRenderPipeline(render_pipeline_id)); } - - let mut life_lock = device.lock_life(); - life_lock - .suspected_resources - .render_pipelines - .insert(pipeline.tracker_index(), pipeline.clone()); - - life_lock - .suspected_resources - .pipeline_layouts - .insert(pipeline.layout.tracker_index(), pipeline.layout.clone()); } } @@ -1877,23 +1793,11 @@ impl Global { let hub = A::hub(self); - if let Some(pipeline) = hub.compute_pipelines.unregister(compute_pipeline_id) { - let device = &pipeline.device; - + if let Some(_pipeline) = hub.compute_pipelines.unregister(compute_pipeline_id) { #[cfg(feature = "trace")] - if let Some(t) = device.trace.lock().as_mut() { + if let Some(t) = _pipeline.device.trace.lock().as_mut() { t.add(trace::Action::DestroyComputePipeline(compute_pipeline_id)); } - - let mut life_lock = device.lock_life(); - life_lock - .suspected_resources - .compute_pipelines - .insert(pipeline.tracker_index(), pipeline.clone()); - life_lock - .suspected_resources - .pipeline_layouts - .insert(pipeline.layout.tracker_index(), pipeline.layout.clone()); } } diff --git a/wgpu-core/src/device/life.rs b/wgpu-core/src/device/life.rs index 7eaaab35a..4ef57e4d1 100644 --- a/wgpu-core/src/device/life.rs +++ b/wgpu-core/src/device/life.rs @@ -1,111 +1,19 @@ use crate::{ - binding_model::{BindGroup, BindGroupLayout, PipelineLayout}, - command::RenderBundle, device::{ queue::{EncoderInFlight, SubmittedWorkDoneClosure, TempResource}, DeviceError, DeviceLostClosure, }, hal_api::HalApi, id, - lock::Mutex, - pipeline::{ComputePipeline, RenderPipeline}, - resource::{self, Buffer, Labeled, QuerySet, Sampler, Texture, TextureView, Trackable}, + resource::{self, Buffer, Labeled, Trackable}, snatch::SnatchGuard, - track::{ResourceTracker, Tracker, TrackerIndex}, - FastHashMap, SubmissionIndex, + SubmissionIndex, }; use smallvec::SmallVec; use std::sync::Arc; use thiserror::Error; -/// A struct that keeps lists of resources that are no longer needed by the user. -pub(crate) struct ResourceMaps { - pub 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 { - pub(crate) fn new() -> Self { - ResourceMaps { - 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) { - let ResourceMaps { - buffers, - textures, - texture_views, - samplers, - bind_groups, - bind_group_layouts, - render_pipelines, - compute_pipelines, - pipeline_layouts, - render_bundles, - query_sets, - } = self; - 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, other: &mut Self) { - let ResourceMaps { - 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()); - 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()); - } -} - /// A command submitted to the GPU for execution. /// /// ## Keeping resources alive while the GPU is using them @@ -113,30 +21,8 @@ impl ResourceMaps { /// [`wgpu_hal`] requires that, when a command is submitted to a queue, all the /// resources it uses must remain alive until it has finished executing. /// -/// The natural way to satisfy this would be for `ActiveSubmission` to hold -/// strong references to all the resources used by its commands. However, that -/// would entail dropping those strong references every time a queue submission -/// finishes, adjusting the reference counts of all the resources it used. This -/// is usually needless work: it's rare for the active submission queue to be -/// the final reference to an object. Usually the user is still holding on to -/// it. -/// -/// To avoid this, an `ActiveSubmission` does not initially hold any strong -/// references to its commands' resources. Instead, each resource tracks the -/// most recent submission index at which it has been used in -/// [`ResourceInfo::submission_index`]. When the user drops a resource, if the -/// submission in which it was last used is still present in the device's queue, -/// we add the resource to [`ActiveSubmission::last_resources`]. Finally, when -/// this `ActiveSubmission` is dequeued and dropped in -/// [`LifetimeTracker::triage_submissions`], we drop `last_resources` along with -/// it. Thus, unless a resource is dropped by the user, it doesn't need to be -/// touched at all when processing completed work. -/// -/// However, it's not clear that this is effective. See [#5560]. -/// /// [`wgpu_hal`]: hal /// [`ResourceInfo::submission_index`]: crate::resource::ResourceInfo -/// [#5560]: https://github.com/gfx-rs/wgpu/issues/5560 struct ActiveSubmission { /// The index of the submission we track. /// @@ -144,16 +30,6 @@ struct ActiveSubmission { /// submission has completed. index: SubmissionIndex, - /// Resources to be freed once this queue submission has completed. - /// - /// When the device is polled, for completed submissions, - /// `triage_submissions` removes resources that don't need to be held alive any longer - /// from there. - /// - /// This includes 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, - /// Temporary resources to be freed once this queue submission has completed. temp_resources: Vec>, @@ -230,10 +106,6 @@ pub(crate) struct LifetimeTracker { /// queue submissions still in flight. mapped: Vec>>, - /// 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, - /// Resources used by queue submissions still in flight. One entry per /// submission, with older submissions appearing before younger. /// @@ -262,7 +134,6 @@ impl LifetimeTracker { pub fn new() -> Self { Self { mapped: Vec::new(), - suspected_resources: ResourceMaps::new(), active: Vec::new(), ready_to_map: Vec::new(), work_done_closures: SmallVec::new(), @@ -284,7 +155,6 @@ impl LifetimeTracker { ) { self.active.push(ActiveSubmission { index, - last_resources: ResourceMaps::new(), temp_resources: temp_resources.collect(), mapped: Vec::new(), encoders, @@ -305,14 +175,10 @@ impl LifetimeTracker { /// [`self.ready_to_map`], where [`LifetimeTracker::handle_mapping`] /// will find them. /// - /// - Resources whose final use was in those submissions are now ready to - /// free. Dropping the submission's [`last_resources`] table does so. - /// /// Return a list of [`SubmittedWorkDoneClosure`]s to run. /// /// [`mapped`]: ActiveSubmission::mapped /// [`self.ready_to_map`]: LifetimeTracker::ready_to_map - /// [`last_resources`]: ActiveSubmission::last_resources /// [`SubmittedWorkDoneClosure`]: crate::device::queue::SubmittedWorkDoneClosure #[must_use] pub fn triage_submissions( @@ -374,307 +240,6 @@ impl LifetimeTracker { } impl LifetimeTracker { - /// Remove abandoned resources from `suspected_resources` and return them. - /// - /// Consult `trackers` to see which resources in `suspected_resources` are - /// abandoned (that is, referenced only by `suspected_resources` and - /// `trackers` itself) and remove them from `suspected_resources`. - /// - /// If the abandoned resources are in use by a command submission still in - /// flight, as listed in `active`, add them to that submission's - /// `ActiveSubmission::last_resources` map. - /// - /// Use `get_resource_map` to find the appropriate member of - /// `ActiveSubmission::last_resources` to hold resources of type `R`. - /// - /// Return a vector of all the abandoned resources that were removed. - fn triage_resources( - suspected_resources: &mut FastHashMap>, - active: &mut [ActiveSubmission], - trackers: &mut impl ResourceTracker, - get_resource_map: impl Fn(&mut ResourceMaps) -> &mut FastHashMap>, - ) -> Vec> - where - R: Trackable, - { - let mut removed_resources = Vec::new(); - suspected_resources.retain(|&index, resource| { - if !trackers.remove_abandoned(index) { - return true; - } - - // If this resource is used by commands in flight, save - // it in that submission's `last_resources` list. - let submit_index = resource.submission_index(); - let last_resources = active - .iter_mut() - .find(|a| a.index == submit_index) - .map(|a| &mut a.last_resources); - if let Some(last_resources) = last_resources { - get_resource_map(last_resources).insert(index, resource.clone()); - } - - removed_resources.push(resource.clone()); - false - }); - removed_resources - } - - fn triage_suspected_render_bundles(&mut self, trackers: &Mutex>) -> &mut Self { - let mut trackers = trackers.lock(); - let suspected_render_bundles = &mut self.suspected_resources.render_bundles; - let removed_resources = Self::triage_resources( - suspected_render_bundles, - self.active.as_mut_slice(), - &mut trackers.bundles, - |maps| &mut maps.render_bundles, - ); - for bundle in removed_resources { - for v in bundle.used.buffers.write().drain_resources() { - self.suspected_resources - .buffers - .insert(v.tracker_index(), v); - } - for v in bundle.used.textures.write().drain_resources() { - self.suspected_resources - .textures - .insert(v.tracker_index(), v); - } - for v in bundle.used.bind_groups.write().drain_resources() { - self.suspected_resources - .bind_groups - .insert(v.tracker_index(), v); - } - for v in bundle.used.render_pipelines.write().drain_resources() { - self.suspected_resources - .render_pipelines - .insert(v.tracker_index(), v); - } - for v in bundle.used.query_sets.write().drain_resources() { - self.suspected_resources - .query_sets - .insert(v.tracker_index(), v); - } - } - self - } - - fn triage_suspected_bind_groups(&mut self, trackers: &Mutex>) -> &mut Self { - let mut trackers = trackers.lock(); - let suspected_bind_groups = &mut self.suspected_resources.bind_groups; - let removed_resources = Self::triage_resources( - suspected_bind_groups, - self.active.as_mut_slice(), - &mut trackers.bind_groups, - |maps| &mut maps.bind_groups, - ); - for bind_group in removed_resources { - for v in bind_group.used.buffers.drain_resources() { - self.suspected_resources - .buffers - .insert(v.tracker_index(), v); - } - for v in bind_group.used.textures.drain_resources() { - self.suspected_resources - .textures - .insert(v.tracker_index(), v); - } - for v in bind_group.used.views.drain_resources() { - self.suspected_resources - .texture_views - .insert(v.tracker_index(), v); - } - for v in bind_group.used.samplers.drain_resources() { - self.suspected_resources - .samplers - .insert(v.tracker_index(), v); - } - - self.suspected_resources - .bind_group_layouts - .insert(bind_group.layout.tracker_index(), bind_group.layout.clone()); - } - self - } - - fn triage_suspected_texture_views(&mut self, trackers: &Mutex>) -> &mut Self { - let mut trackers = trackers.lock(); - let suspected_texture_views = &mut self.suspected_resources.texture_views; - Self::triage_resources( - suspected_texture_views, - self.active.as_mut_slice(), - &mut trackers.views, - |maps| &mut maps.texture_views, - ); - // You might be tempted to add the view's parent texture to - // suspected_resources here, but don't. Texture views get dropped all - // the time, and once a texture is added to - // `LifetimeTracker::suspected_resources` it remains there until it's - // actually dropped, which for long-lived textures could be at the end - // of execution. - self - } - - fn triage_suspected_textures(&mut self, trackers: &Mutex>) -> &mut Self { - let mut trackers = trackers.lock(); - let suspected_textures = &mut self.suspected_resources.textures; - Self::triage_resources( - suspected_textures, - self.active.as_mut_slice(), - &mut trackers.textures, - |maps| &mut maps.textures, - ); - self - } - - fn triage_suspected_samplers(&mut self, trackers: &Mutex>) -> &mut Self { - let mut trackers = trackers.lock(); - let suspected_samplers = &mut self.suspected_resources.samplers; - Self::triage_resources( - suspected_samplers, - self.active.as_mut_slice(), - &mut trackers.samplers, - |maps| &mut maps.samplers, - ); - self - } - - fn triage_suspected_buffers(&mut self, trackers: &Mutex>) -> &mut Self { - let mut trackers = trackers.lock(); - let suspected_buffers = &mut self.suspected_resources.buffers; - Self::triage_resources( - suspected_buffers, - self.active.as_mut_slice(), - &mut trackers.buffers, - |maps| &mut maps.buffers, - ); - self - } - - fn triage_suspected_compute_pipelines(&mut self, trackers: &Mutex>) -> &mut Self { - let mut trackers = trackers.lock(); - let suspected_compute_pipelines = &mut self.suspected_resources.compute_pipelines; - let removed_resources = Self::triage_resources( - suspected_compute_pipelines, - self.active.as_mut_slice(), - &mut trackers.compute_pipelines, - |maps| &mut maps.compute_pipelines, - ); - for compute_pipeline in removed_resources { - self.suspected_resources.pipeline_layouts.insert( - compute_pipeline.layout.tracker_index(), - compute_pipeline.layout.clone(), - ); - } - self - } - - fn triage_suspected_render_pipelines(&mut self, trackers: &Mutex>) -> &mut Self { - let mut trackers = trackers.lock(); - let suspected_render_pipelines = &mut self.suspected_resources.render_pipelines; - let removed_resources = Self::triage_resources( - suspected_render_pipelines, - self.active.as_mut_slice(), - &mut trackers.render_pipelines, - |maps| &mut maps.render_pipelines, - ); - for render_pipeline in removed_resources { - self.suspected_resources.pipeline_layouts.insert( - render_pipeline.layout.tracker_index(), - render_pipeline.layout.clone(), - ); - } - self - } - - fn triage_suspected_pipeline_layouts(&mut self) -> &mut Self { - let mut removed_resources = Vec::new(); - self.suspected_resources - .pipeline_layouts - .retain(|_pipeline_layout_id, pipeline_layout| { - removed_resources.push(pipeline_layout.clone()); - false - }); - removed_resources.drain(..).for_each(|pipeline_layout| { - for bgl in &pipeline_layout.bind_group_layouts { - self.suspected_resources - .bind_group_layouts - .insert(bgl.tracker_index(), bgl.clone()); - } - }); - self - } - - fn triage_suspected_bind_group_layouts(&mut self) -> &mut Self { - //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 - self.suspected_resources.bind_group_layouts.clear(); - - self - } - - fn triage_suspected_query_sets(&mut self, trackers: &Mutex>) -> &mut Self { - let mut trackers = trackers.lock(); - let suspected_query_sets = &mut self.suspected_resources.query_sets; - Self::triage_resources( - suspected_query_sets, - self.active.as_mut_slice(), - &mut trackers.query_sets, - |maps| &mut maps.query_sets, - ); - self - } - - /// Identify resources to free, according to `trackers` and `self.suspected_resources`. - /// - /// Remove from `trackers`, the [`Tracker`] belonging to same [`Device`] as - /// `self`, each resource mentioned in [`self.suspected_resources`]. If - /// `trackers` held the final reference to that resource, add it to the - /// appropriate free list, to be destroyed by the hal: - /// - /// - Add resources used by queue submissions still in flight to the - /// [`last_resources`] table of the last such submission's entry in - /// [`self.active`]. When that submission has finished execution. the - /// [`triage_submissions`] method will remove from the tracker and the - /// resource reference count will be responsible carrying out deallocation. - /// - /// ## Entrained resources - /// - /// This function finds resources that are used only by other resources - /// ready to be freed, and adds those to the free lists as well. For - /// example, if there's some texture `T` used only by some texture view - /// `TV`, then if `TV` can be freed, `T` gets added to the free lists too. - /// - /// Since `wgpu-core` resource ownership patterns are acyclic, we can visit - /// each type that can be owned after all types that could possibly own - /// it. This way, we can detect all free-able objects in a single pass, - /// simply by starting with types that are roots of the ownership DAG (like - /// render bundles) and working our way towards leaf types (like buffers). - /// - /// [`Device`]: super::Device - /// [`self.suspected_resources`]: LifetimeTracker::suspected_resources - /// [`last_resources`]: ActiveSubmission::last_resources - /// [`self.active`]: LifetimeTracker::active - /// [`triage_submissions`]: LifetimeTracker::triage_submissions - pub(crate) fn triage_suspected(&mut self, trackers: &Mutex>) { - profiling::scope!("triage_suspected"); - - // NOTE: The order in which resource types are processed here is - // crucial. See "Entrained resources" in this function's doc comment. - self.triage_suspected_render_bundles(trackers); - self.triage_suspected_compute_pipelines(trackers); - self.triage_suspected_render_pipelines(trackers); - self.triage_suspected_bind_groups(trackers); - self.triage_suspected_pipeline_layouts(); - self.triage_suspected_bind_group_layouts(); - self.triage_suspected_query_sets(trackers); - self.triage_suspected_samplers(trackers); - self.triage_suspected_texture_views(trackers); - self.triage_suspected_textures(trackers); - self.triage_suspected_buffers(trackers); - } - /// Determine which buffers are ready to map, and which must wait for the /// GPU. /// diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index 864702717..444205aac 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -23,7 +23,7 @@ use crate::{ pool::ResourcePool, resource::{ self, Buffer, Labeled, ParentDevice, QuerySet, Sampler, Texture, TextureView, - TextureViewNotRenderableReason, Trackable, TrackingData, + TextureViewNotRenderableReason, TrackingData, }, resource_log, snatch::{SnatchGuard, SnatchLock, Snatchable}, @@ -54,7 +54,6 @@ use std::{ }; use super::{ - life::ResourceMaps, queue::{self, Queue}, DeviceDescriptor, DeviceError, UserClosures, ENTRYPOINT_FAILURE_ERROR, ZERO_BUFFER_SIZE, }; @@ -129,10 +128,6 @@ pub struct Device { #[cfg(feature = "trace")] pub(crate) trace: Mutex>, pub(crate) usage_scopes: UsageScopePool, - - /// Temporary storage, cleared at the start of every call, - /// retained only to save allocations. - temp_suspected: Mutex>>, } pub(crate) enum DeferredDestroy { @@ -269,7 +264,6 @@ impl Device { trackers: Mutex::new(rank::DEVICE_TRACKERS, Tracker::new()), tracker_indices: TrackerIndexAllocators::new(), life_tracker: Mutex::new(rank::DEVICE_LIFE_TRACKER, LifetimeTracker::new()), - temp_suspected: Mutex::new(rank::DEVICE_TEMP_SUSPECTED, Some(ResourceMaps::new())), bgl_pool: ResourcePool::new(), #[cfg(feature = "trace")] trace: Mutex::new( @@ -426,8 +420,6 @@ impl Device { let submission_closures = life_tracker.triage_submissions(last_done_index, &self.command_allocator); - life_tracker.triage_suspected(&self.trackers); - life_tracker.triage_mapped(); let mapping_closures = life_tracker.handle_mapping(self.raw(), &snatch_guard); @@ -474,82 +466,6 @@ impl Device { Ok((closures, queue_empty)) } - pub(crate) fn untrack(&self, trackers: &Tracker) { - // If we have a previously allocated `ResourceMap`, just use that. - let mut temp_suspected = self - .temp_suspected - .lock() - .take() - .unwrap_or_else(|| ResourceMaps::new()); - temp_suspected.clear(); - - // As the tracker is cleared/dropped, we need to consider all the resources - // that it references for destruction in the next GC pass. - { - for resource in trackers.buffers.used_resources() { - if resource.is_unique() { - temp_suspected - .buffers - .insert(resource.tracker_index(), resource.clone()); - } - } - for resource in trackers.textures.used_resources() { - if resource.is_unique() { - temp_suspected - .textures - .insert(resource.tracker_index(), resource.clone()); - } - } - for resource in trackers.views.used_resources() { - if resource.is_unique() { - temp_suspected - .texture_views - .insert(resource.tracker_index(), resource.clone()); - } - } - for resource in trackers.bind_groups.used_resources() { - if resource.is_unique() { - temp_suspected - .bind_groups - .insert(resource.tracker_index(), resource.clone()); - } - } - for resource in trackers.samplers.used_resources() { - if resource.is_unique() { - temp_suspected - .samplers - .insert(resource.tracker_index(), resource.clone()); - } - } - for resource in trackers.compute_pipelines.used_resources() { - if resource.is_unique() { - temp_suspected - .compute_pipelines - .insert(resource.tracker_index(), resource.clone()); - } - } - for resource in trackers.render_pipelines.used_resources() { - if resource.is_unique() { - temp_suspected - .render_pipelines - .insert(resource.tracker_index(), resource.clone()); - } - } - for resource in trackers.query_sets.used_resources() { - if resource.is_unique() { - temp_suspected - .query_sets - .insert(resource.tracker_index(), resource.clone()); - } - } - } - self.lock_life() - .suspected_resources - .extend(&mut temp_suspected); - // Save this resource map for later reuse. - *self.temp_suspected.lock() = Some(temp_suspected); - } - pub(crate) fn create_buffer( self: &Arc, desc: &resource::BufferDescriptor, diff --git a/wgpu-core/src/lock/rank.rs b/wgpu-core/src/lock/rank.rs index 4387b8d13..c01a621aa 100644 --- a/wgpu-core/src/lock/rank.rs +++ b/wgpu-core/src/lock/rank.rs @@ -87,11 +87,6 @@ macro_rules! define_lock_ranks { } define_lock_ranks! { - rank DEVICE_TEMP_SUSPECTED "Device::temp_suspected" followed by { - SHARED_TRACKER_INDEX_ALLOCATOR_INNER, - COMMAND_BUFFER_DATA, - DEVICE_TRACKERS, - } rank COMMAND_BUFFER_DATA "CommandBuffer::data" followed by { DEVICE_SNATCHABLE_LOCK, DEVICE_USAGE_SCOPES, @@ -123,8 +118,6 @@ define_lock_ranks! { } rank DEVICE_LIFE_TRACKER "Device::life_tracker" followed by { COMMAND_ALLOCATOR_FREE_ENCODERS, - // Uncomment this to see an interesting cycle. - // DEVICE_TEMP_SUSPECTED, DEVICE_TRACE, } rank COMMAND_ALLOCATOR_FREE_ENCODERS "CommandAllocator::free_encoders" followed by { diff --git a/wgpu-core/src/resource.rs b/wgpu-core/src/resource.rs index 846882875..e00a942a9 100644 --- a/wgpu-core/src/resource.rs +++ b/wgpu-core/src/resource.rs @@ -200,10 +200,6 @@ pub(crate) trait Trackable: Labeled { /// given index. fn use_at(&self, submit_index: SubmissionIndex); fn submission_index(&self) -> SubmissionIndex; - - fn is_unique(self: &Arc) -> bool { - Arc::strong_count(self) == 1 - } } #[macro_export] diff --git a/wgpu-core/src/track/buffer.rs b/wgpu-core/src/track/buffer.rs index 8a899ef85..6f912b83b 100644 --- a/wgpu-core/src/track/buffer.rs +++ b/wgpu-core/src/track/buffer.rs @@ -7,15 +7,14 @@ use std::{marker::PhantomData, sync::Arc}; -use super::{PendingTransition, ResourceTracker, TrackerIndex}; +use super::{PendingTransition, TrackerIndex}; use crate::{ hal_api::HalApi, lock::{rank, Mutex}, resource::{Buffer, Trackable}, - resource_log, snatch::SnatchGuard, track::{ - invalid_resource_state, skip_barrier, Labeled, ResourceMetadata, ResourceMetadataProvider, + invalid_resource_state, skip_barrier, ResourceMetadata, ResourceMetadataProvider, ResourceUsageCompatibilityError, ResourceUses, }, }; @@ -77,16 +76,6 @@ impl BufferBindGroupState { .into_iter() } - /// Returns a list of all buffers tracked. May contain duplicates. - pub fn drain_resources(&self) -> impl Iterator>> + '_ { - let mut buffers = self.buffers.lock(); - buffers - .drain(..) - .map(|(buffer, _u)| buffer) - .collect::>() - .into_iter() - } - /// Adds the given resource with the given state. pub fn add_single(&self, buffer: &Arc>, state: BufferUses) { let mut buffers = self.buffers.lock(); @@ -136,13 +125,6 @@ impl BufferUsageScope { } } - /// Drains all buffers tracked. - pub fn drain_resources(&mut self) -> impl Iterator>> + '_ { - let resources = self.metadata.drain_resources(); - self.state.clear(); - resources.into_iter() - } - /// Merge the list of buffer states in the given bind group into this usage scope. /// /// If any of the resulting states is invalid, stops the merge and returns a usage @@ -263,67 +245,6 @@ pub(crate) struct BufferTracker { temp: Vec>, } -impl ResourceTracker for BufferTracker { - /// Try to remove the buffer `id` from this tracker if it is otherwise unused. - /// - /// A buffer is 'otherwise unused' when the only references to it are: - /// - /// 1) the `Arc` that our caller, `LifetimeTracker::triage_resources`, is - /// considering draining from `LifetimeTracker::suspected_resources`, - /// - /// 2) its `Arc` in [`self.metadata`] (owned by [`Device::trackers`]), and - /// - /// 3) its `Arc` in the [`Hub::buffers`] registry. - /// - /// If the buffer is indeed unused, this function removes 2), and - /// `triage_suspected` will remove 3), leaving 1) as the sole - /// remaining reference. - /// - /// Returns true if the resource was removed or if not existing in metadata. - /// - /// [`Device::trackers`]: crate::device::Device - /// [`self.metadata`]: BufferTracker::metadata - /// [`Hub::buffers`]: crate::hub::Hub::buffers - fn remove_abandoned(&mut self, index: TrackerIndex) -> bool { - let index = index.as_usize(); - - if index > self.metadata.size() { - return false; - } - - self.tracker_assert_in_bounds(index); - - unsafe { - if self.metadata.contains_unchecked(index) { - let existing_ref_count = self.metadata.get_ref_count_unchecked(index); - //RefCount 2 means that resource is hold just by DeviceTracker and this suspected resource itself - //so it's already been released from user and so it's not inside Registry\Storage - if existing_ref_count <= 2 { - resource_log!( - "BufferTracker::remove_abandoned: removing {}", - self.metadata.get_resource_unchecked(index).error_ident() - ); - - self.metadata.remove(index); - return true; - } - - resource_log!( - "BufferTracker::remove_abandoned: not removing {}, ref count {}", - self.metadata.get_resource_unchecked(index).error_ident(), - existing_ref_count - ); - - return false; - } - } - - resource_log!("BufferTracker::remove_abandoned: does not contain index {index:?}",); - - true - } -} - impl BufferTracker { pub fn new() -> Self { Self { diff --git a/wgpu-core/src/track/metadata.rs b/wgpu-core/src/track/metadata.rs index cc9e29783..1256e0d90 100644 --- a/wgpu-core/src/track/metadata.rs +++ b/wgpu-core/src/track/metadata.rs @@ -116,17 +116,6 @@ impl ResourceMetadata { } } - /// Get the reference count of the resource with the given index. - /// - /// # Safety - /// - /// The given `index` must be in bounds for this `ResourceMetadata`'s - /// existing tables. See `tracker_assert_in_bounds`. - #[inline(always)] - pub(super) unsafe fn get_ref_count_unchecked(&self, index: usize) -> usize { - unsafe { Arc::strong_count(self.get_resource_unchecked(index)) } - } - /// Returns an iterator over the resources owned by `self`. pub(super) fn owned_resources(&self) -> impl Iterator> + '_ { if !self.owned.is_empty() { @@ -138,21 +127,6 @@ impl ResourceMetadata { }) } - /// Returns an iterator over the resources owned by `self`. - pub(super) fn drain_resources(&mut self) -> Vec> { - if !self.owned.is_empty() { - self.tracker_assert_in_bounds(self.owned.len() - 1) - }; - let mut resources = Vec::new(); - iterate_bitvec_indices(&self.owned).for_each(|index| { - let resource = unsafe { self.resources.get_unchecked(index) }; - resources.push(resource.as_ref().unwrap().clone()); - }); - self.owned.clear(); - self.resources.clear(); - resources - } - /// Returns an iterator over the indices of all resources owned by `self`. pub(super) fn owned_indices(&self) -> impl Iterator + '_ { if !self.owned.is_empty() { diff --git a/wgpu-core/src/track/mod.rs b/wgpu-core/src/track/mod.rs index 25ca9dfb2..8d920b383 100644 --- a/wgpu-core/src/track/mod.rs +++ b/wgpu-core/src/track/mod.rs @@ -598,10 +598,6 @@ impl<'a, A: HalApi> UsageScope<'a, A> { } } -pub(crate) trait ResourceTracker { - fn remove_abandoned(&mut self, index: TrackerIndex) -> bool; -} - /// A full double sided tracker used by CommandBuffers and the Device. pub(crate) struct Tracker { pub buffers: BufferTracker, diff --git a/wgpu-core/src/track/stateless.rs b/wgpu-core/src/track/stateless.rs index 903dc2196..a47286dcd 100644 --- a/wgpu-core/src/track/stateless.rs +++ b/wgpu-core/src/track/stateless.rs @@ -9,12 +9,9 @@ use std::sync::Arc; use crate::{ lock::{rank, Mutex}, resource::Trackable, - resource_log, track::ResourceMetadata, }; -use super::{ResourceTracker, TrackerIndex}; - /// Stores all the resources that a bind group stores. #[derive(Debug)] pub(crate) struct StatelessBindGroupState { @@ -43,12 +40,6 @@ impl StatelessBindGroupState { resources.iter().cloned().collect::>().into_iter() } - /// Returns a list of all resources tracked. May contain duplicates. - pub fn drain_resources(&self) -> impl Iterator> + '_ { - let mut resources = self.resources.lock(); - resources.drain(..).collect::>().into_iter() - } - /// Adds the given resource. pub fn add_single(&self, resource: &Arc) { let mut resources = self.resources.lock(); @@ -62,59 +53,6 @@ pub(crate) struct StatelessTracker { metadata: ResourceMetadata, } -impl ResourceTracker for StatelessTracker { - /// Try to remove the given resource from the tracker iff we have the last reference to the - /// resource and the epoch matches. - /// - /// Returns true if the resource was removed or if not existing in metadata. - /// - /// If the ID is higher than the length of internal vectors, - /// false will be returned. - fn remove_abandoned(&mut self, index: TrackerIndex) -> bool { - let index = index.as_usize(); - - if index >= self.metadata.size() { - return false; - } - - self.tracker_assert_in_bounds(index); - - unsafe { - if self.metadata.contains_unchecked(index) { - let existing_ref_count = self.metadata.get_ref_count_unchecked(index); - //RefCount 2 means that resource is hold just by DeviceTracker and this suspected resource itself - //so it's already been released from user and so it's not inside Registry\Storage - if existing_ref_count <= 2 { - resource_log!( - "StatelessTracker<{}>::remove_abandoned: removing {}", - T::TYPE, - self.metadata.get_resource_unchecked(index).error_ident() - ); - - self.metadata.remove(index); - return true; - } - - resource_log!( - "StatelessTracker<{}>::remove_abandoned: not removing {}, ref count {}", - T::TYPE, - self.metadata.get_resource_unchecked(index).error_ident(), - existing_ref_count - ); - - return false; - } - } - - resource_log!( - "StatelessTracker<{}>::remove_abandoned: does not contain index {index:?}", - T::TYPE, - ); - - true - } -} - impl StatelessTracker { pub fn new() -> Self { Self { @@ -146,12 +84,6 @@ impl StatelessTracker { self.metadata.owned_resources() } - /// Returns a list of all resources tracked. - pub fn drain_resources(&mut self) -> impl Iterator> + '_ { - let resources = self.metadata.drain_resources(); - resources.into_iter() - } - /// Inserts a single resource into the resource tracker. /// /// If the resource already exists in the tracker, it will be overwritten. diff --git a/wgpu-core/src/track/texture.rs b/wgpu-core/src/track/texture.rs index 742af97c5..476551624 100644 --- a/wgpu-core/src/track/texture.rs +++ b/wgpu-core/src/track/texture.rs @@ -19,14 +19,11 @@ * will treat the contents as junk. !*/ -use super::{ - range::RangedStates, PendingTransition, PendingTransitionList, ResourceTracker, TrackerIndex, -}; +use super::{range::RangedStates, PendingTransition, PendingTransitionList, TrackerIndex}; use crate::{ hal_api::HalApi, lock::{rank, Mutex}, - resource::{Labeled, Texture, TextureInner, Trackable}, - resource_log, + resource::{Texture, TextureInner, Trackable}, snatch::SnatchGuard, track::{ invalid_resource_state, skip_barrier, ResourceMetadata, ResourceMetadataProvider, @@ -178,16 +175,6 @@ impl TextureBindGroupState { textures.sort_unstable_by_key(|v| v.texture.tracker_index()); } - /// Returns a list of all textures tracked. May contain duplicates. - pub fn drain_resources(&self) -> impl Iterator>> + '_ { - let mut textures = self.textures.lock(); - textures - .drain(..) - .map(|v| v.texture) - .collect::>() - .into_iter() - } - /// Adds the given resource with the given state. pub fn add_single( &self, @@ -274,13 +261,6 @@ impl TextureUsageScope { self.metadata.set_size(size); } - /// Drains all textures tracked. - pub(crate) fn drain_resources(&mut self) -> impl Iterator>> + '_ { - let resources = self.metadata.drain_resources(); - self.set.clear(); - resources.into_iter() - } - /// Returns true if the tracker owns no resources. /// /// This is a O(n) operation. @@ -402,56 +382,6 @@ pub(crate) struct TextureTracker { _phantom: PhantomData, } -impl ResourceTracker for TextureTracker { - /// Try to remove the given resource from the tracker iff we have the last reference to the - /// resource and the epoch matches. - /// - /// Returns true if the resource was removed or if not existing in metadata. - /// - /// If the ID is higher than the length of internal vectors, - /// false will be returned. - fn remove_abandoned(&mut self, index: TrackerIndex) -> bool { - let index = index.as_usize(); - - if index >= self.metadata.size() { - return false; - } - - self.tracker_assert_in_bounds(index); - - unsafe { - if self.metadata.contains_unchecked(index) { - let existing_ref_count = self.metadata.get_ref_count_unchecked(index); - //RefCount 2 means that resource is hold just by DeviceTracker and this suspected resource itself - //so it's already been released from user and so it's not inside Registry\Storage - if existing_ref_count <= 2 { - resource_log!( - "TextureTracker::remove_abandoned: removing {}", - self.metadata.get_resource_unchecked(index).error_ident() - ); - - self.start_set.complex.remove(&index); - self.end_set.complex.remove(&index); - self.metadata.remove(index); - return true; - } - - resource_log!( - "TextureTracker::remove_abandoned: not removing {}, ref count {}", - self.metadata.get_resource_unchecked(index).error_ident(), - existing_ref_count - ); - - return false; - } - } - - resource_log!("TextureTracker::remove_abandoned: does not contain index {index:?}",); - - true - } -} - impl TextureTracker { pub fn new() -> Self { Self {