Remove latest_submission_index (#5976)

* Remove latest_submission_index

* CI

* Comments

Author: cwfitzgerald

benches/Cargo.toml

@@ -43,4 +43,4 @@ pollster.workspace = true
 profiling.workspace = true
 rayon.workspace = true
 tracy-client = { workspace = true, optional = true }
-wgpu = { workspace = true, features = ["wgsl"] }
+wgpu = { workspace = true, features = ["wgsl", "metal", "dx12"] }

wgpu-core/src/device/global.rs

@@ -19,7 +19,6 @@ use crate::{
     present,
     resource::{
         self, BufferAccessError, BufferAccessResult, BufferMapOperation, CreateBufferError,
-        Trackable,
     },
     storage::Storage,
     Label,
@@ -260,15 +259,25 @@ impl Global {
     ) -> Result<(), WaitIdleError> {
         let hub = A::hub(self);
 
-        let last_submission = match hub.buffers.read().get(buffer_id) {
-            Ok(buffer) => buffer.submission_index(),
+        let device = hub
+            .devices
+            .get(device_id)
+            .map_err(|_| DeviceError::InvalidDeviceId)?;
+
+        let buffer = match hub.buffers.get(buffer_id) {
+            Ok(buffer) => buffer,
             Err(_) => return Ok(()),
         };
 
-        hub.devices
-            .get(device_id)
-            .map_err(|_| DeviceError::InvalidDeviceId)?
-            .wait_for_submit(last_submission)
+        let last_submission = device
+            .lock_life()
+            .get_buffer_latest_submission_index(&buffer);
+
+        if let Some(last_submission) = last_submission {
+            device.wait_for_submit(last_submission)
+        } else {
+            Ok(())
+        }
     }
 
     #[doc(hidden)]
@@ -424,7 +433,13 @@ impl Global {
         );
 
         if wait {
-            let last_submit_index = buffer.submission_index();
+            let Some(last_submit_index) = buffer
+                .device
+                .lock_life()
+                .get_buffer_latest_submission_index(&buffer)
+            else {
+                return;
+            };
             match buffer.device.wait_for_submit(last_submit_index) {
                 Ok(()) => (),
                 Err(e) => log::error!("Failed to wait for buffer {:?}: {}", buffer_id, e),
@@ -599,7 +614,13 @@ impl Global {
             }
 
             if wait {
-                let last_submit_index = texture.submission_index();
+                let Some(last_submit_index) = texture
+                    .device
+                    .lock_life()
+                    .get_texture_latest_submission_index(&texture)
+                else {
+                    return;
+                };
                 match texture.device.wait_for_submit(last_submit_index) {
                     Ok(()) => (),
                     Err(e) => log::error!("Failed to wait for texture {texture_id:?}: {e}"),
@@ -672,7 +693,13 @@ impl Global {
             }
 
             if wait {
-                let last_submit_index = view.submission_index();
+                let Some(last_submit_index) = view
+                    .device
+                    .lock_life()
+                    .get_texture_latest_submission_index(&view.parent)
+                else {
+                    return Ok(());
+                };
                 match view.device.wait_for_submit(last_submit_index) {
                     Ok(()) => (),
                     Err(e) => {

wgpu-core/src/device/life.rs

@@ -5,7 +5,7 @@ use crate::{
     },
     hal_api::HalApi,
     id,
-    resource::{self, Buffer, Labeled, Trackable},
+    resource::{self, Buffer, Labeled, Texture, Trackable},
     snatch::SnatchGuard,
     SubmissionIndex,
 };
@@ -55,6 +55,58 @@ struct ActiveSubmission<A: HalApi> {
     work_done_closures: SmallVec<[SubmittedWorkDoneClosure; 1]>,
 }
 
+impl<A: HalApi> ActiveSubmission<A> {
+    /// Returns true if this submission contains the given buffer.
+    ///
+    /// This only uses constant-time operations.
+    pub fn contains_buffer(&self, buffer: &Buffer<A>) -> bool {
+        for encoder in &self.encoders {
+            // The ownership location of buffers depends on where the command encoder
+            // came from. If it is the staging command encoder on the queue, it is
+            // in the pending buffer list. If it came from a user command encoder,
+            // it is in the tracker.
+
+            if encoder.trackers.buffers.contains(buffer) {
+                return true;
+            }
+
+            if encoder
+                .pending_buffers
+                .contains_key(&buffer.tracker_index())
+            {
+                return true;
+            }
+        }
+
+        false
+    }
+
+    /// Returns true if this submission contains the given texture.
+    ///
+    /// This only uses constant-time operations.
+    pub fn contains_texture(&self, texture: &Texture<A>) -> bool {
+        for encoder in &self.encoders {
+            // The ownership location of textures depends on where the command encoder
+            // came from. If it is the staging command encoder on the queue, it is
+            // in the pending buffer list. If it came from a user command encoder,
+            // it is in the tracker.
+
+            if encoder.trackers.textures.contains(texture) {
+                return true;
+            }
+
+            if encoder
+                .pending_textures
+                .contains_key(&texture.tracker_index())
+            {
+                return true;
+            }
+        }
+
+        false
+    }
+}
+
 #[derive(Clone, Debug, Error)]
 #[non_exhaustive]
 pub enum WaitIdleError {
@@ -165,6 +217,40 @@ impl<A: HalApi> LifetimeTracker<A> {
         self.mapped.push(value.clone());
     }
 
+    /// Returns the submission index of the most recent submission that uses the
+    /// given buffer.
+    pub fn get_buffer_latest_submission_index(
+        &self,
+        buffer: &Buffer<A>,
+    ) -> Option<SubmissionIndex> {
+        // We iterate in reverse order, so that we can bail out early as soon
+        // as we find a hit.
+        self.active.iter().rev().find_map(|submission| {
+            if submission.contains_buffer(buffer) {
+                Some(submission.index)
+            } else {
+                None
+            }
+        })
+    }
+
+    /// Returns the submission index of the most recent submission that uses the
+    /// given texture.
+    pub fn get_texture_latest_submission_index(
+        &self,
+        texture: &Texture<A>,
+    ) -> Option<SubmissionIndex> {
+        // We iterate in reverse order, so that we can bail out early as soon
+        // as we find a hit.
+        self.active.iter().rev().find_map(|submission| {
+            if submission.contains_texture(texture) {
+                Some(submission.index)
+            } else {
+                None
+            }
+        })
+    }
+
     /// Sort out the consequences of completed submissions.
     ///
     /// Assume that all submissions up through `last_done` have completed.
@@ -236,9 +322,7 @@ impl<A: HalApi> LifetimeTracker<A> {
             }
         }
     }
-}
 
-impl<A: HalApi> LifetimeTracker<A> {
     /// Determine which buffers are ready to map, and which must wait for the
     /// GPU.
     ///
@@ -249,17 +333,19 @@ impl<A: HalApi> LifetimeTracker<A> {
         }
 
         for buffer in self.mapped.drain(..) {
-            let submit_index = buffer.submission_index();
+            let submission = self
+                .active
+                .iter_mut()
+                .rev()
+                .find(|a| a.contains_buffer(&buffer));
+
             log::trace!(
-                "Mapping of {} at submission {:?} gets assigned to active {:?}",
+                "Mapping of {} at submission {:?}",
                 buffer.error_ident(),
-                submit_index,
-                self.active.iter().position(|a| a.index == submit_index)
+                submission.as_deref().map(|s| s.index)
             );
 
-            self.active
-                .iter_mut()
-                .find(|a| a.index == submit_index)
+            submission
                 .map_or(&mut self.ready_to_map, |a| &mut a.mapped)
                 .push(buffer);
         }

wgpu-core/src/device/queue.rs

@@ -149,12 +149,12 @@ pub enum TempResource<A: HalApi> {
 pub(crate) struct EncoderInFlight<A: HalApi> {
     raw: A::CommandEncoder,
     cmd_buffers: Vec<A::CommandBuffer>,
-    trackers: Tracker<A>,
+    pub(crate) trackers: Tracker<A>,
 
     /// These are the buffers that have been tracked by `PendingWrites`.
-    pending_buffers: Vec<Arc<Buffer<A>>>,
+    pub(crate) pending_buffers: FastHashMap<TrackerIndex, Arc<Buffer<A>>>,
     /// These are the textures that have been tracked by `PendingWrites`.
-    pending_textures: Vec<Arc<Texture<A>>>,
+    pub(crate) pending_textures: FastHashMap<TrackerIndex, Arc<Texture<A>>>,
 }
 
 impl<A: HalApi> EncoderInFlight<A> {
@@ -268,8 +268,8 @@ impl<A: HalApi> PendingWrites<A> {
         queue: &A::Queue,
     ) -> Result<Option<EncoderInFlight<A>>, DeviceError> {
         if self.is_recording {
-            let pending_buffers = self.dst_buffers.drain().map(|(_, b)| b).collect();
-            let pending_textures = self.dst_textures.drain().map(|(_, t)| t).collect();
+            let pending_buffers = mem::take(&mut self.dst_buffers);
+            let pending_textures = mem::take(&mut self.dst_textures);
 
             let cmd_buf = unsafe { self.command_encoder.end_encoding()? };
             self.is_recording = false;
@@ -570,8 +570,6 @@ impl Global {
 
         self.queue_validate_write_buffer_impl(&dst, buffer_offset, staging_buffer.size)?;
 
-        dst.use_at(device.active_submission_index.load(Ordering::Relaxed) + 1);
-
         let region = hal::BufferCopy {
             src_offset: 0,
             dst_offset: buffer_offset,
@@ -762,7 +760,6 @@ impl Global {
         // call above. Since we've held `texture_guard` the whole time, we know
         // the texture hasn't gone away in the mean time, so we can unwrap.
         let dst = hub.textures.get(destination.texture).unwrap();
-        dst.use_at(device.active_submission_index.load(Ordering::Relaxed) + 1);
 
         let dst_raw = dst.try_raw(&snatch_guard)?;
 
@@ -1007,7 +1004,6 @@ impl Global {
                     .drain(init_layer_range);
             }
         }
-        dst.use_at(device.active_submission_index.load(Ordering::Relaxed) + 1);
 
         let snatch_guard = device.snatchable_lock.read();
         let dst_raw = dst.try_raw(&snatch_guard)?;
@@ -1126,7 +1122,7 @@ impl Global {
                         }
 
                         {
-                            profiling::scope!("update submission ids");
+                            profiling::scope!("check resource state");
 
                             let cmd_buf_data = cmdbuf.data.lock();
                             let cmd_buf_trackers = &cmd_buf_data.as_ref().unwrap().trackers;
@@ -1136,7 +1132,6 @@ impl Global {
                                 profiling::scope!("buffers");
                                 for buffer in cmd_buf_trackers.buffers.used_resources() {
                                     buffer.check_destroyed(&snatch_guard)?;
-                                    buffer.use_at(submit_index);
 
                                     match *buffer.map_state.lock() {
                                         BufferMapState::Idle => (),
@@ -1163,7 +1158,6 @@ impl Global {
                                             true
                                         }
                                     };
-                                    texture.use_at(submit_index);
                                     if should_extend {
                                         unsafe {
                                             used_surface_textures
@@ -1177,69 +1171,6 @@ impl Global {
                                     }
                                 }
                             }
-                            {
-                                profiling::scope!("views");
-                                for texture_view in cmd_buf_trackers.views.used_resources() {
-                                    texture_view.use_at(submit_index);
-                                }
-                            }
-                            {
-                                profiling::scope!("bind groups (+ referenced views/samplers)");
-                                for bg in cmd_buf_trackers.bind_groups.used_resources() {
-                                    bg.use_at(submit_index);
-                                    // We need to update the submission indices for the contained
-                                    // state-less (!) resources as well, so that they don't get
-                                    // deleted too early if the parent bind group goes out of scope.
-                                    for view in bg.used.views.used_resources() {
-                                        view.use_at(submit_index);
-                                    }
-                                    for sampler in bg.used.samplers.used_resources() {
-                                        sampler.use_at(submit_index);
-                                    }
-                                }
-                            }
-                            {
-                                profiling::scope!("compute pipelines");
-                                for compute_pipeline in
-                                    cmd_buf_trackers.compute_pipelines.used_resources()
-                                {
-                                    compute_pipeline.use_at(submit_index);
-                                }
-                            }
-                            {
-                                profiling::scope!("render pipelines");
-                                for render_pipeline in
-                                    cmd_buf_trackers.render_pipelines.used_resources()
-                                {
-                                    render_pipeline.use_at(submit_index);
-                                }
-                            }
-                            {
-                                profiling::scope!("query sets");
-                                for query_set in cmd_buf_trackers.query_sets.used_resources() {
-                                    query_set.use_at(submit_index);
-                                }
-                            }
-                            {
-                                profiling::scope!(
-                                    "render bundles (+ referenced pipelines/query sets)"
-                                );
-                                for bundle in cmd_buf_trackers.bundles.used_resources() {
-                                    bundle.use_at(submit_index);
-                                    // We need to update the submission indices for the contained
-                                    // state-less (!) resources as well, excluding the bind groups.
-                                    // They don't get deleted too early if the bundle goes out of scope.
-                                    for render_pipeline in
-                                        bundle.used.render_pipelines.read().used_resources()
-                                    {
-                                        render_pipeline.use_at(submit_index);
-                                    }
-                                    for query_set in bundle.used.query_sets.read().used_resources()
-                                    {
-                                        query_set.use_at(submit_index);
-                                    }
-                                }
-                            }
                         }
                         let mut baked = cmdbuf.from_arc_into_baked();
 
@@ -1303,8 +1234,8 @@ impl Global {
                             raw: baked.encoder,
                             cmd_buffers: baked.list,
                             trackers: baked.trackers,
-                            pending_buffers: Vec::new(),
-                            pending_textures: Vec::new(),
+                            pending_buffers: FastHashMap::default(),
+                            pending_textures: FastHashMap::default(),
                         });
                     }