From 85b91c04080f05437e310a1b4f5dd6909c21830e Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Wed, 20 Dec 2023 22:42:42 +0100 Subject: [PATCH] Buffer snatching (#4896) Co-authored-by: Connor Fitzgerald --- wgpu-core/src/device/life.rs | 44 +++++++++++++++++++++++++- wgpu-core/src/device/queue.rs | 5 +-- wgpu-core/src/resource.rs | 59 ++++++++++++++++++++++++++++++----- 3 files changed, 97 insertions(+), 11 deletions(-) diff --git a/wgpu-core/src/device/life.rs b/wgpu-core/src/device/life.rs index ce6edadb4c..5fb3ad225b 100644 --- a/wgpu-core/src/device/life.rs +++ b/wgpu-core/src/device/life.rs @@ -14,7 +14,10 @@ use crate::{ TextureViewId, }, pipeline::{ComputePipeline, RenderPipeline}, - resource::{self, Buffer, QuerySet, Resource, Sampler, StagingBuffer, Texture, TextureView}, + resource::{ + self, Buffer, DestroyedBuffer, QuerySet, Resource, Sampler, StagingBuffer, Texture, + TextureView, + }, track::{ResourceTracker, Tracker}, FastHashMap, SubmissionIndex, }; @@ -39,6 +42,7 @@ pub(crate) struct ResourceMaps { pub pipeline_layouts: FastHashMap>>, pub render_bundles: FastHashMap>>, pub query_sets: FastHashMap>>, + pub destroyed_buffers: FastHashMap>>, } impl ResourceMaps { @@ -56,6 +60,7 @@ impl ResourceMaps { pipeline_layouts: FastHashMap::default(), render_bundles: FastHashMap::default(), query_sets: FastHashMap::default(), + destroyed_buffers: FastHashMap::default(), } } @@ -73,6 +78,7 @@ impl ResourceMaps { pipeline_layouts, render_bundles, query_sets, + destroyed_buffers, } = self; buffers.clear(); staging_buffers.clear(); @@ -86,6 +92,7 @@ impl ResourceMaps { pipeline_layouts.clear(); render_bundles.clear(); query_sets.clear(); + destroyed_buffers.clear(); } pub(crate) fn extend(&mut self, mut other: Self) { @@ -102,6 +109,7 @@ impl ResourceMaps { pipeline_layouts, render_bundles, query_sets, + destroyed_buffers, } = self; buffers.extend(other.buffers.drain()); staging_buffers.extend(other.staging_buffers.drain()); @@ -115,6 +123,7 @@ impl ResourceMaps { pipeline_layouts.extend(other.pipeline_layouts.drain()); render_bundles.extend(other.render_bundles.drain()); query_sets.extend(other.query_sets.drain()); + destroyed_buffers.extend(other.destroyed_buffers.drain()); } } @@ -281,6 +290,11 @@ impl LifetimeTracker { .staging_buffers .insert(raw.as_info().id(), raw); } + TempResource::DestroyedBuffer(destroyed) => { + last_resources + .destroyed_buffers + .insert(destroyed.id, destroyed); + } TempResource::Texture(raw) => { last_resources.textures.insert(raw.as_info().id(), raw); } @@ -384,6 +398,9 @@ impl LifetimeTracker { 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); } @@ -642,6 +659,27 @@ impl LifetimeTracker { self } + fn triage_suspected_destroyed_buffers( + &mut self, + #[cfg(feature = "trace")] trace: &mut Option<&mut trace::Trace>, + ) { + for (id, buffer) in self.suspected_resources.destroyed_buffers.drain() { + let submit_index = buffer.submission_index; + if let Some(resources) = self.active.iter_mut().find(|a| a.index == submit_index) { + resources + .last_resources + .destroyed_buffers + .insert(id, buffer); + } else { + self.free_resources.destroyed_buffers.insert(id, buffer); + } + #[cfg(feature = "trace")] + if let Some(ref mut t) = *trace { + t.add(trace::Action::DestroyBuffer(id)); + } + } + } + fn triage_suspected_compute_pipelines( &mut self, trackers: &Mutex>, @@ -871,6 +909,10 @@ impl LifetimeTracker { #[cfg(feature = "trace")] &mut trace, ); + self.triage_suspected_destroyed_buffers( + #[cfg(feature = "trace")] + &mut trace, + ); } /// Determine which buffers are ready to map, and which must wait for the diff --git a/wgpu-core/src/device/queue.rs b/wgpu-core/src/device/queue.rs index 2a7e4a77db..5dff3a19ab 100644 --- a/wgpu-core/src/device/queue.rs +++ b/wgpu-core/src/device/queue.rs @@ -16,8 +16,8 @@ use crate::{ identity::{GlobalIdentityHandlerFactory, Input}, init_tracker::{has_copy_partial_init_tracker_coverage, TextureInitRange}, resource::{ - Buffer, BufferAccessError, BufferMapState, Resource, ResourceInfo, ResourceType, - StagingBuffer, Texture, TextureInner, + Buffer, BufferAccessError, BufferMapState, DestroyedBuffer, Resource, ResourceInfo, + ResourceType, StagingBuffer, Texture, TextureInner, }, resource_log, track, FastHashMap, SubmissionIndex, }; @@ -163,6 +163,7 @@ pub struct WrappedSubmissionIndex { pub enum TempResource { Buffer(Arc>), StagingBuffer(Arc>), + DestroyedBuffer(Arc>), Texture(Arc>), } diff --git a/wgpu-core/src/resource.rs b/wgpu-core/src/resource.rs index 217d9dd4c1..8f0607e7f9 100644 --- a/wgpu-core/src/resource.rs +++ b/wgpu-core/src/resource.rs @@ -413,7 +413,7 @@ pub struct Buffer { impl Drop for Buffer { fn drop(&mut self) { if let Some(raw) = self.raw.take() { - resource_log!("Destroy raw Buffer {:?}", self.info.label()); + resource_log!("Deallocate raw Buffer (dropped) {:?}", self.info.label()); unsafe { use hal::Device; self.device.raw().destroy_buffer(raw); @@ -551,14 +551,25 @@ impl Buffer { if let Some(ref mut trace) = *device.trace.lock() { trace.add(trace::Action::FreeBuffer(buffer_id)); } - // Note: a future commit will replace this with a read guard - // and actually snatch the buffer. - let snatch_guard = device.snatchable_lock.read(); - if self.raw.get(&snatch_guard).is_none() { - return Err(resource::DestroyError::AlreadyDestroyed); - } - let temp = queue::TempResource::Buffer(self.clone()); + let temp = { + let snatch_guard = device.snatchable_lock.write(); + let raw = match self.raw.snatch(snatch_guard) { + Some(raw) => raw, + None => { + return Err(resource::DestroyError::AlreadyDestroyed); + } + }; + + queue::TempResource::DestroyedBuffer(Arc::new(DestroyedBuffer { + raw: Some(raw), + device: Arc::clone(&self.device), + submission_index: self.info.submission_index(), + id: self.info.id.unwrap(), + label: self.info.label.clone(), + })) + }; + let mut pending_writes = device.pending_writes.lock(); let pending_writes = pending_writes.as_mut().unwrap(); if pending_writes.dst_buffers.contains_key(&buffer_id) { @@ -605,6 +616,38 @@ impl Resource for Buffer { } } +/// A buffer that has been marked as destroyed and is staged for actual deletion soon. +#[derive(Debug)] +pub struct DestroyedBuffer { + raw: Option, + device: Arc>, + label: String, + pub(crate) id: BufferId, + pub(crate) submission_index: u64, +} + +impl DestroyedBuffer { + pub fn label(&self) -> &dyn Debug { + if !self.label.is_empty() { + return &self.label; + } + + &self.id + } +} + +impl Drop for DestroyedBuffer { + fn drop(&mut self) { + if let Some(raw) = self.raw.take() { + resource_log!("Deallocate raw Buffer (destroyed) {:?}", self.label()); + unsafe { + use hal::Device; + self.device.raw().destroy_buffer(raw); + } + } + } +} + /// A temporary buffer, consumed by the command that uses it. /// /// A [`StagingBuffer`] is designed for one-shot uploads of data to the GPU. It