Remove the free_resources tracker (#5037)

Author: nical

wgpu-core/src/device/life.rs

@@ -142,9 +142,8 @@ struct ActiveSubmission<A: HalApi> {
     /// Resources to be freed once this queue submission has completed.
     ///
     /// When the device is polled, for completed submissions,
-    /// `triage_submissions` merges these into
-    /// `LifetimeTracker::free_resources`. From there,
-    /// `LifetimeTracker::cleanup` passes them to the hal to be freed.
+    /// `triage_submissions` removes resources that don't need to be held alive any longer
+    /// from there.
     ///
     /// This includes things like temporary resources and resources that are
     /// used by submitted commands but have been dropped by the user (meaning that
@@ -205,8 +204,6 @@ pub enum WaitIdleError {
 ///         buffers that were dropped by the user get moved to
 ///         `self.free_resources`.
 ///
-///     4)  `cleanup` frees everything in `free_resources`.
-///
 /// Only calling `Global::buffer_map_async` clones a new `Arc` for the
 /// buffer. This new `Arc` is only dropped by `handle_mapping`.
 pub(crate) struct LifetimeTracker<A: HalApi> {
@@ -233,13 +230,6 @@ pub(crate) struct LifetimeTracker<A: HalApi> {
     /// to particular entries.
     active: Vec<ActiveSubmission<A>>,
 
-    /// Raw backend resources that are neither referenced nor used.
-    ///
-    /// 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<A>,
-
     /// Buffers the user has asked us to map, and which are not used by any
     /// queue submission still in flight.
     ready_to_map: Vec<Arc<Buffer<A>>>,
@@ -264,7 +254,6 @@ impl<A: HalApi> LifetimeTracker<A> {
             future_suspected_textures: Vec::new(),
             suspected_resources: ResourceMaps::new(),
             active: Vec::new(),
-            free_resources: ResourceMaps::new(),
             ready_to_map: Vec::new(),
             work_done_closures: SmallVec::new(),
             device_lost_closure: None,
@@ -374,7 +363,6 @@ impl<A: HalApi> LifetimeTracker<A> {
         let mut work_done_closures: SmallVec<_> = self.work_done_closures.drain(..).collect();
         for a in self.active.drain(..done_count) {
             log::debug!("Active submission {} is done", a.index);
-            self.free_resources.extend(a.last_resources);
             self.ready_to_map.extend(a.mapped);
             for encoder in a.encoders {
                 let raw = unsafe { encoder.land() };
@@ -385,11 +373,6 @@ impl<A: HalApi> LifetimeTracker<A> {
         work_done_closures
     }
 
-    pub fn cleanup(&mut self) {
-        profiling::scope!("LifetimeTracker::cleanup");
-        self.free_resources.clear();
-    }
-
     pub fn schedule_resource_destruction(
         &mut self,
         temp_resource: TempResource<A>,
@@ -399,22 +382,24 @@ impl<A: HalApi> LifetimeTracker<A> {
             .active
             .iter_mut()
             .find(|a| a.index == last_submit_index)
-            .map_or(&mut self.free_resources, |a| &mut a.last_resources);
-        match temp_resource {
-            TempResource::Buffer(raw) => {
-                resources.buffers.insert(raw.as_info().id(), raw);
-            }
-            TempResource::StagingBuffer(raw) => {
-                resources.staging_buffers.insert(raw.as_info().id(), raw);
-            }
-            TempResource::DestroyedBuffer(destroyed) => {
-                resources.destroyed_buffers.insert(destroyed.id, destroyed);
-            }
-            TempResource::Texture(raw) => {
-                resources.textures.insert(raw.as_info().id(), raw);
-            }
-            TempResource::DestroyedTexture(destroyed) => {
-                resources.destroyed_textures.insert(destroyed.id, destroyed);
+            .map(|a| &mut a.last_resources);
+        if let Some(resources) = resources {
+            match temp_resource {
+                TempResource::Buffer(raw) => {
+                    resources.buffers.insert(raw.as_info().id(), raw);
+                }
+                TempResource::StagingBuffer(raw) => {
+                    resources.staging_buffers.insert(raw.as_info().id(), raw);
+                }
+                TempResource::DestroyedBuffer(destroyed) => {
+                    resources.destroyed_buffers.insert(destroyed.id, destroyed);
+                }
+                TempResource::Texture(raw) => {
+                    resources.textures.insert(raw.as_info().id(), raw);
+                }
+                TempResource::DestroyedTexture(destroyed) => {
+                    resources.destroyed_textures.insert(destroyed.id, destroyed);
+                }
             }
         }
     }
@@ -437,7 +422,6 @@ impl<A: HalApi> LifetimeTracker<A> {
     fn triage_resources<Id, R>(
         resources_map: &mut FastHashMap<Id, Arc<R>>,
         active: &mut [ActiveSubmission<A>],
-        free_resources: &mut ResourceMaps<A>,
         trackers: &mut impl ResourceTracker<Id, R>,
         get_resource_map: impl Fn(&mut ResourceMaps<A>) -> &mut FastHashMap<Id, Arc<R>>,
     ) -> Vec<Arc<R>>
@@ -451,12 +435,14 @@ impl<A: HalApi> LifetimeTracker<A> {
             let non_referenced_resources = active
                 .iter_mut()
                 .find(|a| a.index == submit_index)
-                .map_or(&mut *free_resources, |a| &mut a.last_resources);
+                .map(|a| &mut a.last_resources);
 
             let is_removed = trackers.remove_abandoned(id);
             if is_removed {
                 removed_resources.push(resource.clone());
-                get_resource_map(non_referenced_resources).insert(id, resource.clone());
+                if let Some(ressources) = non_referenced_resources {
+                    get_resource_map(ressources).insert(id, resource.clone());
+                }
             }
             !is_removed
         });
@@ -469,7 +455,6 @@ impl<A: HalApi> LifetimeTracker<A> {
         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,
         );
@@ -507,7 +492,6 @@ impl<A: HalApi> LifetimeTracker<A> {
         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,
         );
@@ -544,7 +528,6 @@ impl<A: HalApi> LifetimeTracker<A> {
         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,
         );
@@ -565,7 +548,6 @@ impl<A: HalApi> LifetimeTracker<A> {
         Self::triage_resources(
             resource_map,
             self.active.as_mut_slice(),
-            &mut self.free_resources,
             &mut trackers.textures,
             |maps| &mut maps.textures,
         );
@@ -578,7 +560,6 @@ impl<A: HalApi> LifetimeTracker<A> {
         Self::triage_resources(
             resource_map,
             self.active.as_mut_slice(),
-            &mut self.free_resources,
             &mut trackers.samplers,
             |maps| &mut maps.samplers,
         );
@@ -588,23 +569,13 @@ impl<A: HalApi> LifetimeTracker<A> {
     fn triage_suspected_buffers(&mut self, trackers: &Mutex<Tracker<A>>) -> &mut Self {
         let mut trackers = trackers.lock();
         let resource_map = &mut self.suspected_resources.buffers;
-        let mut removed_resources = Self::triage_resources(
+        Self::triage_resources(
             resource_map,
             self.active.as_mut_slice(),
-            &mut self.free_resources,
             &mut trackers.buffers,
             |maps| &mut maps.buffers,
         );
-        removed_resources.drain(..).for_each(|buffer| {
-            if let resource::BufferMapState::Init {
-                ref stage_buffer, ..
-            } = *buffer.map_state.lock()
-            {
-                self.free_resources
-                    .buffers
-                    .insert(stage_buffer.as_info().id(), stage_buffer.clone());
-            }
-        });
+
         self
     }
 
@@ -616,8 +587,6 @@ impl<A: HalApi> LifetimeTracker<A> {
                     .last_resources
                     .destroyed_buffers
                     .insert(id, buffer);
-            } else {
-                self.free_resources.destroyed_buffers.insert(id, buffer);
             }
         }
     }
@@ -630,8 +599,6 @@ impl<A: HalApi> LifetimeTracker<A> {
                     .last_resources
                     .destroyed_textures
                     .insert(id, texture);
-            } else {
-                self.free_resources.destroyed_textures.insert(id, texture);
             }
         }
     }
@@ -642,7 +609,6 @@ impl<A: HalApi> LifetimeTracker<A> {
         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,
         );
@@ -661,7 +627,6 @@ impl<A: HalApi> LifetimeTracker<A> {
         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,
         );
@@ -693,18 +658,11 @@ impl<A: HalApi> LifetimeTracker<A> {
     }
 
     fn triage_suspected_bind_group_layouts(&mut self) -> &mut Self {
-        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
-                // encounter could drop the refcount to 0.
-                self.free_resources
-                    .bind_group_layouts
-                    .insert(*bind_group_layout_id, bind_group_layout.clone());
-                false
-            },
-        );
+        //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
     }
 
@@ -714,22 +672,15 @@ impl<A: HalApi> LifetimeTracker<A> {
         Self::triage_resources(
             resource_map,
             self.active.as_mut_slice(),
-            &mut self.free_resources,
             &mut trackers.query_sets,
             |maps| &mut maps.query_sets,
         );
         self
     }
 
     fn triage_suspected_staging_buffers(&mut self) -> &mut Self {
-        self.suspected_resources
-            .staging_buffers
-            .retain(|staging_buffer_id, staging_buffer| {
-                self.free_resources
-                    .staging_buffers
-                    .insert(*staging_buffer_id, staging_buffer.clone());
-                false
-            });
+        self.suspected_resources.staging_buffers.clear();
+
         self
     }
 
@@ -746,12 +697,8 @@ impl<A: HalApi> LifetimeTracker<A> {
     /// -   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 move them to
-    ///     [`self.free_resources`].
-    ///
-    /// -   Add resources that can be freed right now to [`self.free_resources`]
-    ///     directly. [`LifetimeTracker::cleanup`] will take care of them as
-    ///     part of this poll.
+    ///     [`triage_submissions`] method will remove from the tracker and the
+    ///     resource reference count will be responsible carrying out deallocation.
     ///
     /// ## Entrained resources
     ///
@@ -771,7 +718,6 @@ impl<A: HalApi> LifetimeTracker<A> {
     /// [`last_resources`]: ActiveSubmission::last_resources
     /// [`self.active`]: LifetimeTracker::active
     /// [`triage_submissions`]: LifetimeTracker::triage_submissions
-    /// [`self.free_resources`]: LifetimeTracker::free_resources
     pub(crate) fn triage_suspected(&mut self, trackers: &Mutex<Tracker<A>>) {
         profiling::scope!("triage_suspected");
 
@@ -844,9 +790,6 @@ impl<A: HalApi> LifetimeTracker<A> {
             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
-                    .buffers
-                    .insert(buffer_id, buffer.clone());
             } else {
                 let mapping = match std::mem::replace(
                     &mut *buffer.map_state.lock(),

wgpu-core/src/device/resource.rs

@@ -372,9 +372,6 @@ impl<A: HalApi> Device<A> {
         );
         let mapping_closures = life_tracker.handle_mapping(self.raw(), &self.trackers);
 
-        //Cleaning up resources and released all unused suspected ones
-        life_tracker.cleanup();
-
         // Detect if we have been destroyed and now need to lose the device.
         // If we are invalid (set at start of destroy) and our queue is empty,
         // and we have a DeviceLostClosure, return the closure to be called by
@@ -3383,7 +3380,6 @@ impl<A: HalApi> Device<A> {
             current_index,
             self.command_allocator.lock().as_mut().unwrap(),
         );
-        life_tracker.cleanup();
         #[cfg(feature = "trace")]
         {
             *self.trace.lock() = None;