From 8ba5c828313d8564d3a16fa4ea7ce7c6a575bae8 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Thu, 17 Oct 2024 14:10:14 +0200 Subject: [PATCH] minimize moves by introducing `WeakVec` --- wgpu-core/src/device/resource.rs | 75 +++++++++++++++----------------- wgpu-core/src/lib.rs | 1 + wgpu-core/src/resource.rs | 35 ++++++++------- wgpu-core/src/weak_vec.rs | 67 ++++++++++++++++++++++++++++ 4 files changed, 120 insertions(+), 58 deletions(-) create mode 100644 wgpu-core/src/weak_vec.rs diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index 14fa26aa6..21ecf85d2 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -31,6 +31,7 @@ use crate::{ UsageScopePool, }, validation::{self, validate_color_attachment_bytes_per_sample}, + weak_vec::WeakVec, FastHashMap, LabelHelpers, PreHashedKey, PreHashedMap, }; @@ -42,7 +43,7 @@ use wgt::{ use std::{ borrow::Cow, - mem::ManuallyDrop, + mem::{self, ManuallyDrop}, num::NonZeroU32, sync::{ atomic::{AtomicBool, AtomicU64, Ordering}, @@ -150,8 +151,8 @@ pub struct Device { } pub(crate) enum DeferredDestroy { - TextureView(Weak), - BindGroup(Weak), + TextureViews(WeakVec), + BindGroups(WeakVec), } impl std::fmt::Debug for Device { @@ -384,36 +385,42 @@ impl Device { /// implementation of a reference-counted structure). /// The snatch lock must not be held while this function is called. pub(crate) fn deferred_resource_destruction(&self) { - while let Some(item) = self.deferred_destroy.lock().pop() { + let deferred_destroy = mem::take(&mut *self.deferred_destroy.lock()); + for item in deferred_destroy { match item { - DeferredDestroy::TextureView(view) => { - let Some(view) = view.upgrade() else { - continue; - }; - let Some(raw_view) = view.raw.snatch(&mut self.snatchable_lock.write()) else { - continue; - }; + DeferredDestroy::TextureViews(views) => { + for view in views { + let Some(view) = view.upgrade() else { + continue; + }; + let Some(raw_view) = view.raw.snatch(&mut self.snatchable_lock.write()) + else { + continue; + }; - resource_log!("Destroy raw {}", view.error_ident()); + resource_log!("Destroy raw {}", view.error_ident()); - unsafe { - self.raw().destroy_texture_view(raw_view); + unsafe { + self.raw().destroy_texture_view(raw_view); + } } } - DeferredDestroy::BindGroup(bind_group) => { - let Some(bind_group) = bind_group.upgrade() else { - continue; - }; - let Some(raw_bind_group) = - bind_group.raw.snatch(&mut self.snatchable_lock.write()) - else { - continue; - }; + DeferredDestroy::BindGroups(bind_groups) => { + for bind_group in bind_groups { + let Some(bind_group) = bind_group.upgrade() else { + continue; + }; + let Some(raw_bind_group) = + bind_group.raw.snatch(&mut self.snatchable_lock.write()) + else { + continue; + }; - resource_log!("Destroy raw {}", bind_group.error_ident()); + resource_log!("Destroy raw {}", bind_group.error_ident()); - unsafe { - self.raw().destroy_bind_group(raw_bind_group); + unsafe { + self.raw().destroy_bind_group(raw_bind_group); + } } } } @@ -638,7 +645,7 @@ impl Device { map_state: Mutex::new(rank::BUFFER_MAP_STATE, resource::BufferMapState::Idle), label: desc.label.to_string(), tracking_data: TrackingData::new(self.tracker_indices.buffers.clone()), - bind_groups: Mutex::new(rank::BUFFER_BIND_GROUPS, Vec::new()), + bind_groups: Mutex::new(rank::BUFFER_BIND_GROUPS, WeakVec::new()), #[cfg(feature = "indirect-validation")] raw_indirect_validation_bind_group, }; @@ -753,7 +760,7 @@ impl Device { map_state: Mutex::new(rank::BUFFER_MAP_STATE, resource::BufferMapState::Idle), label: desc.label.to_string(), tracking_data: TrackingData::new(self.tracker_indices.buffers.clone()), - bind_groups: Mutex::new(rank::BUFFER_BIND_GROUPS, Vec::new()), + bind_groups: Mutex::new(rank::BUFFER_BIND_GROUPS, WeakVec::new()), #[cfg(feature = "indirect-validation")] raw_indirect_validation_bind_group, }; @@ -1386,10 +1393,6 @@ impl Device { { let mut views = texture.views.lock(); - - // Remove stale weak references - views.retain(|view| view.strong_count() > 0); - views.push(Arc::downgrade(&view)); } @@ -2379,18 +2382,10 @@ impl Device { let weak_ref = Arc::downgrade(&bind_group); for range in &bind_group.used_texture_ranges { let mut bind_groups = range.texture.bind_groups.lock(); - - // Remove stale weak references - bind_groups.retain(|bg| bg.strong_count() > 0); - bind_groups.push(weak_ref.clone()); } for range in &bind_group.used_buffer_ranges { let mut bind_groups = range.buffer.bind_groups.lock(); - - // Remove stale weak references - bind_groups.retain(|bg| bg.strong_count() > 0); - bind_groups.push(weak_ref.clone()); } diff --git a/wgpu-core/src/lib.rs b/wgpu-core/src/lib.rs index c85288f47..b87b2cd63 100644 --- a/wgpu-core/src/lib.rs +++ b/wgpu-core/src/lib.rs @@ -81,6 +81,7 @@ pub mod resource; mod snatch; pub mod storage; mod track; +mod weak_vec; // This is public for users who pre-compile shaders while still wanting to // preserve all run-time checks that `wgpu-core` does. // See , after which this can be diff --git a/wgpu-core/src/resource.rs b/wgpu-core/src/resource.rs index 3cd5765bd..bddccd939 100644 --- a/wgpu-core/src/resource.rs +++ b/wgpu-core/src/resource.rs @@ -14,6 +14,7 @@ use crate::{ resource_log, snatch::{SnatchGuard, Snatchable}, track::{SharedTrackerIndexAllocator, TextureSelector, TrackerIndex}, + weak_vec::WeakVec, Label, LabelHelpers, }; @@ -26,7 +27,7 @@ use std::{ mem::{self, ManuallyDrop}, ops::Range, ptr::NonNull, - sync::{Arc, Weak}, + sync::Arc, }; /// Information about the wgpu-core resource. @@ -474,7 +475,7 @@ pub struct Buffer { pub(crate) label: String, pub(crate) tracking_data: TrackingData, pub(crate) map_state: Mutex, - pub(crate) bind_groups: Mutex>>, + pub(crate) bind_groups: Mutex>, #[cfg(feature = "indirect-validation")] pub(crate) raw_indirect_validation_bind_group: Snatchable>, } @@ -824,7 +825,7 @@ pub struct DestroyedBuffer { raw: ManuallyDrop>, device: Arc, label: String, - bind_groups: Vec>, + bind_groups: WeakVec, #[cfg(feature = "indirect-validation")] raw_indirect_validation_bind_group: Option>, } @@ -838,9 +839,9 @@ impl DestroyedBuffer { impl Drop for DestroyedBuffer { fn drop(&mut self) { let mut deferred = self.device.deferred_destroy.lock(); - for bind_group in self.bind_groups.drain(..) { - deferred.push(DeferredDestroy::BindGroup(bind_group)); - } + deferred.push(DeferredDestroy::BindGroups(mem::take( + &mut self.bind_groups, + ))); drop(deferred); #[cfg(feature = "indirect-validation")] @@ -1060,8 +1061,8 @@ pub struct Texture { pub(crate) label: String, pub(crate) tracking_data: TrackingData, pub(crate) clear_mode: TextureClearMode, - pub(crate) views: Mutex>>, - pub(crate) bind_groups: Mutex>>, + pub(crate) views: Mutex>, + pub(crate) bind_groups: Mutex>, } impl Texture { @@ -1095,8 +1096,8 @@ impl Texture { label: desc.label.to_string(), tracking_data: TrackingData::new(device.tracker_indices.textures.clone()), clear_mode, - views: Mutex::new(rank::TEXTURE_VIEWS, Vec::new()), - bind_groups: Mutex::new(rank::TEXTURE_BIND_GROUPS, Vec::new()), + views: Mutex::new(rank::TEXTURE_VIEWS, WeakVec::new()), + bind_groups: Mutex::new(rank::TEXTURE_BIND_GROUPS, WeakVec::new()), } } /// Checks that the given texture usage contains the required texture usage, @@ -1430,8 +1431,8 @@ impl Global { #[derive(Debug)] pub struct DestroyedTexture { raw: ManuallyDrop>, - views: Vec>, - bind_groups: Vec>, + views: WeakVec, + bind_groups: WeakVec, device: Arc, label: String, } @@ -1447,12 +1448,10 @@ impl Drop for DestroyedTexture { let device = &self.device; let mut deferred = device.deferred_destroy.lock(); - for view in self.views.drain(..) { - deferred.push(DeferredDestroy::TextureView(view)); - } - for bind_group in self.bind_groups.drain(..) { - deferred.push(DeferredDestroy::BindGroup(bind_group)); - } + deferred.push(DeferredDestroy::TextureViews(mem::take(&mut self.views))); + deferred.push(DeferredDestroy::BindGroups(mem::take( + &mut self.bind_groups, + ))); drop(deferred); resource_log!("Destroy raw Texture (destroyed) {:?}", self.label()); diff --git a/wgpu-core/src/weak_vec.rs b/wgpu-core/src/weak_vec.rs new file mode 100644 index 000000000..d3e6293ba --- /dev/null +++ b/wgpu-core/src/weak_vec.rs @@ -0,0 +1,67 @@ +//! Module containing the [`WeakVec`] API. + +use std::sync::Weak; + +/// A container that holds Weak references of T. +/// +/// On `push` it scans its contents for weak references with no strong references still alive and drops them. +#[derive(Debug)] +pub(crate) struct WeakVec { + inner: Vec>>, +} + +impl Default for WeakVec { + fn default() -> Self { + Self { + inner: Default::default(), + } + } +} + +impl WeakVec { + pub(crate) fn new() -> Self { + Self { inner: Vec::new() } + } + + /// Pushes a new element to this collection, dropping older elements that no longer have + /// a strong reference to them. + /// + /// NOTE: The length and capacity of this collection do not change when old elements are + /// dropped. + pub(crate) fn push(&mut self, value: Weak) { + let mut to_insert = Some(value); + for slot in &mut self.inner { + if let Some(w) = slot { + if w.strong_count() == 0 { + *slot = to_insert.take(); + } + } else { + *slot = to_insert.take(); + } + } + if let Some(to_insert) = to_insert { + self.inner.push(Some(to_insert)); + } + } +} + +pub(crate) struct WeakVecIter { + inner: std::iter::Flatten>>>, +} + +impl Iterator for WeakVecIter { + type Item = Weak; + fn next(&mut self) -> Option { + self.inner.next() + } +} + +impl IntoIterator for WeakVec { + type Item = Weak; + type IntoIter = WeakVecIter; + fn into_iter(self) -> Self::IntoIter { + WeakVecIter { + inner: self.inner.into_iter().flatten(), + } + } +}