Merge #722

722: Refactor tracking of device last completed submission r=kvark a=kvark

**Connections**
Fixes https://github.com/gfx-rs/wgpu-rs/issues/358

**Description**
We used to track the next submission index in `device.life_guard.submission_index` atomic. This PR changes that to point to the last *done* submission, and also introduces a non-atomic field to keep track of the current/next submission.
This allows us to avoid waiting on the frame semaphore on presentation if the relevant submission is done by then.

**Testing**
Not tested!

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>

wgpu-core/src/device/life.rs

@@ -259,15 +259,6 @@ impl<B: hal::Backend> LifetimeTracker<B> {
         });
     }
 
-    /// Find the pending entry with the lowest active index. If none can be found that means
-    /// everything in the allocator can be cleaned up, so std::usize::MAX is correct.
-    #[cfg(feature = "replay")]
-    pub fn lowest_active_submission(&self) -> SubmissionIndex {
-        self.active
-            .iter()
-            .fold(std::usize::MAX, |v, active| active.index.min(v))
-    }
-
     fn wait_idle(&self, device: &B::Device) {
         if !self.active.is_empty() {
             log::debug!("Waiting for IDLE...");

wgpu-core/src/device/mod.rs

@@ -7,7 +7,7 @@ use crate::{
     hub::{GfxBackend, Global, GlobalIdentityHandlerFactory, Hub, Input, Token},
     id, pipeline, resource, swap_chain,
     track::{BufferState, TextureState, TrackerSet},
-    validation, FastHashMap, LifeGuard, PrivateFeatures, Stored, MAX_BIND_GROUPS,
+    validation, FastHashMap, LifeGuard, PrivateFeatures, Stored, SubmissionIndex, MAX_BIND_GROUPS,
 };
 
 use arrayvec::ArrayVec;
@@ -26,7 +26,7 @@ use wgt::{
 };
 
 use std::{
-    collections::hash_map::Entry, ffi, iter, marker::PhantomData, mem, ptr, slice,
+    collections::hash_map::Entry, ffi, iter, marker::PhantomData, mem, ops::Range, ptr, slice,
     sync::atomic::Ordering,
 };
 
@@ -179,7 +179,9 @@ pub struct Device<B: hal::Backend> {
     pub(crate) com_allocator: command::CommandAllocator<B>,
     mem_allocator: Mutex<Heaps<B>>,
     desc_allocator: Mutex<DescriptorAllocator<B>>,
+    //Note: The submission index here corresponds to the last submission that is done.
     pub(crate) life_guard: LifeGuard,
+    pub(crate) active_submission_index: SubmissionIndex,
     pub(crate) trackers: Mutex<TrackerSet>,
     pub(crate) render_passes: Mutex<FastHashMap<RenderPassKey, B::RenderPass>>,
     pub(crate) framebuffers: Mutex<FastHashMap<FramebufferKey, B::Framebuffer>>,
@@ -210,10 +212,6 @@ impl<B: GfxBackend> Device<B> {
         capabilities: wgt::Capabilities,
         trace_path: Option<&std::path::Path>,
     ) -> Self {
-        // don't start submission index at zero
-        let life_guard = LifeGuard::new();
-        life_guard.submission_index.fetch_add(1, Ordering::Relaxed);
-
         let com_allocator = command::CommandAllocator::new(queue_group.family, &raw);
         let heaps = unsafe {
             Heaps::new(
@@ -242,7 +240,8 @@ impl<B: GfxBackend> Device<B> {
             mem_allocator: Mutex::new(heaps),
             desc_allocator: Mutex::new(DescriptorAllocator::new()),
             queue_group,
-            life_guard,
+            life_guard: LifeGuard::new(),
+            active_submission_index: 0,
             trackers: Mutex::new(TrackerSet::new(B::VARIANT)),
             render_passes: Mutex::new(FastHashMap::default()),
             framebuffers: Mutex::new(FastHashMap::default()),
@@ -271,6 +270,10 @@ impl<B: GfxBackend> Device<B> {
         }
     }
 
+    pub(crate) fn last_completed_submission_index(&self) -> SubmissionIndex {
+        self.life_guard.submission_index.load(Ordering::Acquire)
+    }
+
     fn lock_life_internal<'this, 'token: 'this>(
         tracker: &'this Mutex<life::LifetimeTracker<B>>,
         _token: &mut Token<'token, Self>,
@@ -278,7 +281,7 @@ impl<B: GfxBackend> Device<B> {
         tracker.lock()
     }
 
-    fn lock_life<'this, 'token: 'this>(
+    pub(crate) fn lock_life<'this, 'token: 'this>(
         &'this self,
         token: &mut Token<'token, Self>,
     ) -> MutexGuard<'this, life::LifetimeTracker<B>> {
@@ -306,6 +309,9 @@ impl<B: GfxBackend> Device<B> {
         let callbacks = life_tracker.handle_mapping(hub, &self.raw, &self.trackers, token);
         life_tracker.cleanup(&self.raw, &self.mem_allocator, &self.desc_allocator);
 
+        self.life_guard
+            .submission_index
+            .store(last_done, Ordering::Release);
         self.com_allocator.maintain(&self.raw, last_done);
         callbacks
     }
@@ -766,14 +772,15 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
         };
 
         let device = &device_guard[device_id];
-        let mut life_lock = device.lock_life(&mut token);
-        if life_lock.lowest_active_submission() <= last_submission {
+        if device.last_completed_submission_index() <= last_submission {
             log::info!(
                 "Waiting for submission {:?} before accessing buffer {:?}",
                 last_submission,
                 buffer_id
             );
-            life_lock.triage_submissions(&device.raw, true);
+            device
+                .lock_life(&mut token)
+                .triage_submissions(&device.raw, true);
         }
     }
 
@@ -2558,6 +2565,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             semaphore: device.raw.create_semaphore().unwrap(),
             acquired_view_id: None,
             acquired_framebuffers: Vec::new(),
+            active_submission_index: 0,
         };
         swap_chain_guard.insert(sc_id, swap_chain);
         sc_id
@@ -2644,7 +2652,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
     pub fn buffer_map_async<B: GfxBackend>(
         &self,
         buffer_id: id::BufferId,
-        range: std::ops::Range<BufferAddress>,
+        range: Range<BufferAddress>,
         op: resource::BufferMapOperation,
     ) {
         let hub = B::hub(self);
@@ -2756,8 +2764,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                 };
                 let _ = ptr;
 
-                let last_submit_index = device.life_guard.submission_index.load(Ordering::Relaxed);
-                buffer.life_guard.use_at(last_submit_index + 1);
+                buffer.life_guard.use_at(device.active_submission_index + 1);
                 let region = hal::command::BufferCopy {
                     src: 0,
                     dst: 0,

wgpu-core/src/device/queue.rs

@@ -15,7 +15,7 @@ use crate::{
 use gfx_memory::{Block, Heaps, MemoryBlock};
 use hal::{command::CommandBuffer as _, device::Device as _, queue::CommandQueue as _};
 use smallvec::SmallVec;
-use std::{iter, sync::atomic::Ordering};
+use std::iter;
 
 struct StagingData<B: hal::Backend> {
     buffer: B::Buffer,
@@ -181,8 +181,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             "Write buffer usage {:?} must contain flag COPY_DST",
             dst.usage
         );
-        let last_submit_index = device.life_guard.submission_index.load(Ordering::Relaxed);
-        dst.life_guard.use_at(last_submit_index + 1);
+        dst.life_guard.use_at(device.active_submission_index + 1);
 
         assert_eq!(
             data_size % wgt::COPY_BUFFER_ALIGNMENT,
@@ -329,8 +328,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
         );
         crate::command::validate_texture_copy_range(destination, dst.kind, size);
 
-        let last_submit_index = device.life_guard.submission_index.load(Ordering::Relaxed);
-        dst.life_guard.use_at(last_submit_index + 1);
+        dst.life_guard.use_at(device.active_submission_index + 1);
 
         let region = hal::command::BufferImageCopy {
             buffer_offset: 0,
@@ -385,11 +383,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                         comb_raw
                     });
             device.temp_suspected.clear();
-
-            let submit_index = 1 + device
-                .life_guard
-                .submission_index
-                .fetch_add(1, Ordering::Relaxed);
+            device.active_submission_index += 1;
+            let submit_index = device.active_submission_index;
 
             let fence = {
                 let mut signal_swapchain_semaphores = SmallVec::<[_; 1]>::new();
@@ -425,6 +420,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
 
                         if let Some((sc_id, fbo)) = comb.used_swap_chain.take() {
                             let sc = &mut swap_chain_guard[sc_id.value];
+                            sc.active_submission_index = submit_index;
                             assert!(sc.acquired_view_id.is_some(),
                                 "SwapChainOutput for {:?} was dropped before the respective command buffer {:?} got submitted!",
                                 sc_id.value, cmb_id);

wgpu-core/src/swap_chain.rs

@@ -38,7 +38,7 @@ use crate::{
     conv,
     hub::{GfxBackend, Global, GlobalIdentityHandlerFactory, Input, Token},
     id::{DeviceId, SwapChainId, TextureViewId},
-    resource, LifeGuard, PrivateFeatures, Stored,
+    resource, LifeGuard, PrivateFeatures, Stored, SubmissionIndex,
 };
 
 use hal::{self, device::Device as _, queue::CommandQueue as _, window::PresentationSurface as _};
@@ -56,6 +56,7 @@ pub struct SwapChain<B: hal::Backend> {
     pub(crate) semaphore: B::Semaphore,
     pub(crate) acquired_view_id: Option<Stored<TextureViewId>>,
     pub(crate) acquired_framebuffers: Vec<B::Framebuffer>,
+    pub(crate) active_submission_index: SubmissionIndex,
 }
 
 pub(crate) fn swap_chain_descriptor_to_hal(
@@ -202,9 +203,14 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
             resource::TextureViewInner::SwapChain { image, .. } => image,
         };
 
-        let err = unsafe {
+        let err = {
+            let sem = if sc.active_submission_index > device.last_completed_submission_index() {
+                Some(&sc.semaphore)
+            } else {
+                None
+            };
             let queue = &mut device.queue_group.queues[0];
-            queue.present_surface(B::get_surface_mut(surface), image, Some(&sc.semaphore))
+            unsafe { queue.present_surface(B::get_surface_mut(surface), image, sem) }
         };
         if let Err(e) = err {
             log::warn!("present failed: {:?}", e);