From 1b005bf1d27465dd3710b3809ca6ab760ee3bf60 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Mon, 18 Mar 2024 10:23:48 +0000 Subject: [PATCH 1/8] pool tracker vecs --- wgpu-core/src/track/buffer.rs | 23 +++++++++++++++++++---- wgpu-core/src/track/metadata.rs | 11 +++++++++++ wgpu-core/src/track/mod.rs | 1 + wgpu-core/src/track/texture.rs | 27 +++++++++++++++++++++++---- 4 files changed, 54 insertions(+), 8 deletions(-) diff --git a/wgpu-core/src/track/buffer.rs b/wgpu-core/src/track/buffer.rs index a30ac2a225..4412d08cb1 100644 --- a/wgpu-core/src/track/buffer.rs +++ b/wgpu-core/src/track/buffer.rs @@ -7,7 +7,7 @@ use std::{borrow::Cow, marker::PhantomData, sync::Arc}; -use super::{PendingTransition, ResourceTracker, TrackerIndex}; +use super::{pool::{BitvecPool, VecPool}, PendingTransition, ResourceTracker, TrackerIndex}; use crate::{ hal_api::HalApi, id::BufferId, @@ -104,6 +104,10 @@ impl BufferBindGroupState { } } +static STATE_POOL: VecPool = VecPool::new(); +static RES_POOL: VecPool = VecPool::new(); +static RES_BIT_POOL: BitvecPool = BitvecPool::new(); + /// Stores all buffer state within a single usage scope. #[derive(Debug)] pub(crate) struct BufferUsageScope { @@ -112,12 +116,23 @@ pub(crate) struct BufferUsageScope { metadata: ResourceMetadata>, } +impl Drop for BufferUsageScope { + fn drop(&mut self) { + let (bits, res) = self.metadata.return_vecs(); + RES_BIT_POOL.put(bits); + unsafe { + STATE_POOL.put(&mut self.state); + RES_POOL.put(res); + } + } +} + impl BufferUsageScope { pub fn new() -> Self { Self { - state: Vec::new(), - - metadata: ResourceMetadata::new(), + // safety: not safe if we have multiple HalApis in a single execution + state: unsafe { STATE_POOL.get() }, + metadata: ResourceMetadata::new_with_vecs(RES_BIT_POOL.get(), unsafe { RES_POOL.get() }), } } diff --git a/wgpu-core/src/track/metadata.rs b/wgpu-core/src/track/metadata.rs index 744783a7fa..d8d3d3958a 100644 --- a/wgpu-core/src/track/metadata.rs +++ b/wgpu-core/src/track/metadata.rs @@ -29,6 +29,17 @@ impl ResourceMetadata { } } + pub(super) fn new_with_vecs(bitvec: BitVec, vec: Vec>>) -> Self { + Self { + owned: bitvec, + resources: vec, + } + } + + pub(super) fn return_vecs(&mut self) -> (&mut BitVec, &mut Vec>>) { + (&mut self.owned, &mut self.resources) + } + /// Returns the number of indices we can accommodate. pub(super) fn size(&self) -> usize { self.owned.len() diff --git a/wgpu-core/src/track/mod.rs b/wgpu-core/src/track/mod.rs index 9ca37ebadc..63dd119af4 100644 --- a/wgpu-core/src/track/mod.rs +++ b/wgpu-core/src/track/mod.rs @@ -100,6 +100,7 @@ mod metadata; mod range; mod stateless; mod texture; +pub mod pool; use crate::{ binding_model, command, conv, hal_api::HalApi, id, pipeline, resource, snatch::SnatchGuard, diff --git a/wgpu-core/src/track/texture.rs b/wgpu-core/src/track/texture.rs index e7c4707c93..6dbcf5949e 100644 --- a/wgpu-core/src/track/texture.rs +++ b/wgpu-core/src/track/texture.rs @@ -19,7 +19,7 @@ * will treat the contents as junk. !*/ -use super::{ +use super::{pool::{BitvecPool, VecPool}, range::RangedStates, PendingTransition, PendingTransitionList, ResourceTracker, TrackerIndex, }; use crate::{ @@ -204,16 +204,25 @@ impl TextureBindGroupState { } } +static USES_POOL: VecPool = VecPool::new(); + /// Container for corresponding simple and complex texture states. #[derive(Debug)] pub(crate) struct TextureStateSet { simple: Vec, complex: FastHashMap, } + +impl Drop for TextureStateSet { + fn drop(&mut self) { + unsafe { USES_POOL.put(&mut self.simple) } + } +} + impl TextureStateSet { fn new() -> Self { Self { - simple: Vec::new(), + simple: unsafe { USES_POOL.get() }, complex: FastHashMap::default(), } } @@ -235,12 +244,22 @@ pub(crate) struct TextureUsageScope { metadata: ResourceMetadata>, } +static RES_POOL: VecPool = VecPool::new(); +static RES_BIT_POOL: BitvecPool = BitvecPool::new(); + +impl Drop for TextureUsageScope { + fn drop(&mut self) { + let (bits, res) = self.metadata.return_vecs(); + unsafe { RES_POOL.put(res) }; + RES_BIT_POOL.put(bits); + } +} + impl TextureUsageScope { pub fn new() -> Self { Self { set: TextureStateSet::new(), - - metadata: ResourceMetadata::new(), + metadata: ResourceMetadata::new_with_vecs(RES_BIT_POOL.get(), unsafe { RES_POOL.get() }), } } From 6e5e9a548089cde8d81d194c7e1b8eebd8a78933 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Mon, 18 Mar 2024 12:03:44 +0000 Subject: [PATCH 2/8] pool --- wgpu-core/src/track/pool.rs | 47 +++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 wgpu-core/src/track/pool.rs diff --git a/wgpu-core/src/track/pool.rs b/wgpu-core/src/track/pool.rs new file mode 100644 index 0000000000..b857e2f357 --- /dev/null +++ b/wgpu-core/src/track/pool.rs @@ -0,0 +1,47 @@ +use bit_vec::BitVec; +use once_cell::sync::OnceCell; +use parking_lot::Mutex; +pub struct VecPool { + pool: OnceCell>>>, +} + +impl VecPool { + pub const fn new() -> Self { + Self{ pool: OnceCell::new() } + } + + /// safety: a pool instance must only ever be called with a single type T to ensure transmute is sound + /// (`#[repr(rust)]` does not guarantee equal member order for different monomorphs) + pub unsafe fn get(&self) -> Vec { + unsafe { std::mem::transmute(self.pool.get_or_init(|| Default::default()).lock().pop().unwrap_or_default()) } + } + + /// safety: a pool instance must only ever be called with a single type T to ensure transmute is sound + /// (`#[repr(rust)]` does not guarantee equal member order for different monomorphs) + pub unsafe fn put(&self, vec: &mut Vec) { + let mut vec = std::mem::take(vec); + vec.clear(); + let vec = unsafe { std::mem::transmute(vec) }; + self.pool.get().unwrap().lock().push(vec); + } +} + +pub struct BitvecPool { + pool: OnceCell>>>, +} + +impl BitvecPool { + pub const fn new() -> Self { + Self{ pool: OnceCell::new() } + } + + pub fn get(&self) -> BitVec { + self.pool.get_or_init(|| Default::default()).lock().pop().unwrap_or_default() + } + + pub fn put(&self, vec: &mut BitVec) { + let mut vec = std::mem::take(vec); + vec.clear(); + self.pool.get().unwrap().lock().push(vec); + } +} From ab0ffea19b8d4b756448b5ebc6e2e4f36fccd6cb Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Mon, 18 Mar 2024 12:15:39 +0000 Subject: [PATCH 3/8] ci --- wgpu-core/src/track/buffer.rs | 9 ++- wgpu-core/src/track/mod.rs | 2 +- wgpu-core/src/track/pool.rs | 110 +++++++++++++++++++-------------- wgpu-core/src/track/texture.rs | 10 ++- 4 files changed, 78 insertions(+), 53 deletions(-) diff --git a/wgpu-core/src/track/buffer.rs b/wgpu-core/src/track/buffer.rs index 4412d08cb1..4f73a7f023 100644 --- a/wgpu-core/src/track/buffer.rs +++ b/wgpu-core/src/track/buffer.rs @@ -7,7 +7,10 @@ use std::{borrow::Cow, marker::PhantomData, sync::Arc}; -use super::{pool::{BitvecPool, VecPool}, PendingTransition, ResourceTracker, TrackerIndex}; +use super::{ + pool::{BitvecPool, VecPool}, + PendingTransition, ResourceTracker, TrackerIndex, +}; use crate::{ hal_api::HalApi, id::BufferId, @@ -132,7 +135,9 @@ impl BufferUsageScope { Self { // safety: not safe if we have multiple HalApis in a single execution state: unsafe { STATE_POOL.get() }, - metadata: ResourceMetadata::new_with_vecs(RES_BIT_POOL.get(), unsafe { RES_POOL.get() }), + metadata: ResourceMetadata::new_with_vecs(RES_BIT_POOL.get(), unsafe { + RES_POOL.get() + }), } } diff --git a/wgpu-core/src/track/mod.rs b/wgpu-core/src/track/mod.rs index 63dd119af4..e2216d6283 100644 --- a/wgpu-core/src/track/mod.rs +++ b/wgpu-core/src/track/mod.rs @@ -97,10 +97,10 @@ Device <- CommandBuffer = insert(device.start, device.end, buffer.start, buffer. mod buffer; mod metadata; +pub mod pool; mod range; mod stateless; mod texture; -pub mod pool; use crate::{ binding_model, command, conv, hal_api::HalApi, id, pipeline, resource, snatch::SnatchGuard, diff --git a/wgpu-core/src/track/pool.rs b/wgpu-core/src/track/pool.rs index b857e2f357..c9ba16666a 100644 --- a/wgpu-core/src/track/pool.rs +++ b/wgpu-core/src/track/pool.rs @@ -1,47 +1,63 @@ -use bit_vec::BitVec; -use once_cell::sync::OnceCell; -use parking_lot::Mutex; -pub struct VecPool { - pool: OnceCell>>>, -} - -impl VecPool { - pub const fn new() -> Self { - Self{ pool: OnceCell::new() } - } - - /// safety: a pool instance must only ever be called with a single type T to ensure transmute is sound - /// (`#[repr(rust)]` does not guarantee equal member order for different monomorphs) - pub unsafe fn get(&self) -> Vec { - unsafe { std::mem::transmute(self.pool.get_or_init(|| Default::default()).lock().pop().unwrap_or_default()) } - } - - /// safety: a pool instance must only ever be called with a single type T to ensure transmute is sound - /// (`#[repr(rust)]` does not guarantee equal member order for different monomorphs) - pub unsafe fn put(&self, vec: &mut Vec) { - let mut vec = std::mem::take(vec); - vec.clear(); - let vec = unsafe { std::mem::transmute(vec) }; - self.pool.get().unwrap().lock().push(vec); - } -} - -pub struct BitvecPool { - pool: OnceCell>>>, -} - -impl BitvecPool { - pub const fn new() -> Self { - Self{ pool: OnceCell::new() } - } - - pub fn get(&self) -> BitVec { - self.pool.get_or_init(|| Default::default()).lock().pop().unwrap_or_default() - } - - pub fn put(&self, vec: &mut BitVec) { - let mut vec = std::mem::take(vec); - vec.clear(); - self.pool.get().unwrap().lock().push(vec); - } -} +use bit_vec::BitVec; +use once_cell::sync::OnceCell; +use parking_lot::Mutex; +pub struct VecPool { + pool: OnceCell>>>, +} + +impl VecPool { + pub const fn new() -> Self { + Self { + pool: OnceCell::new(), + } + } + + /// safety: a pool instance must only ever be called with a single type T to ensure transmute is sound + /// (`#[repr(rust)]` does not guarantee equal member order for different monomorphs) + pub unsafe fn get(&self) -> Vec { + unsafe { + std::mem::transmute( + self.pool + .get_or_init(Default::default) + .lock() + .pop() + .unwrap_or_default(), + ) + } + } + + /// safety: a pool instance must only ever be called with a single type T to ensure transmute is sound + /// (`#[repr(rust)]` does not guarantee equal member order for different monomorphs) + pub unsafe fn put(&self, vec: &mut Vec) { + let mut vec = std::mem::take(vec); + vec.clear(); + let vec = unsafe { std::mem::transmute(vec) }; + self.pool.get_or_init(Default::default).lock().push(vec); + } +} + +pub struct BitvecPool { + pool: OnceCell>>>, +} + +impl BitvecPool { + pub const fn new() -> Self { + Self { + pool: OnceCell::new(), + } + } + + pub fn get(&self) -> BitVec { + self.pool + .get_or_init(Default::default) + .lock() + .pop() + .unwrap_or_default() + } + + pub fn put(&self, vec: &mut BitVec) { + let mut vec = std::mem::take(vec); + vec.clear(); + self.pool.get_or_init(Default::default).lock().push(vec); + } +} diff --git a/wgpu-core/src/track/texture.rs b/wgpu-core/src/track/texture.rs index 6dbcf5949e..c447e9bd22 100644 --- a/wgpu-core/src/track/texture.rs +++ b/wgpu-core/src/track/texture.rs @@ -19,8 +19,10 @@ * will treat the contents as junk. !*/ -use super::{pool::{BitvecPool, VecPool}, - range::RangedStates, PendingTransition, PendingTransitionList, ResourceTracker, TrackerIndex, +use super::{ + pool::{BitvecPool, VecPool}, + range::RangedStates, + PendingTransition, PendingTransitionList, ResourceTracker, TrackerIndex, }; use crate::{ hal_api::HalApi, @@ -259,7 +261,9 @@ impl TextureUsageScope { pub fn new() -> Self { Self { set: TextureStateSet::new(), - metadata: ResourceMetadata::new_with_vecs(RES_BIT_POOL.get(), unsafe { RES_POOL.get() }), + metadata: ResourceMetadata::new_with_vecs(RES_BIT_POOL.get(), unsafe { + RES_POOL.get() + }), } } From 06cff9dbdb161bd024ec6fcd1ce81bb2c825eb93 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Tue, 19 Mar 2024 13:25:46 +0000 Subject: [PATCH 4/8] move pool to device --- wgpu-core/src/command/compute.rs | 8 ++-- wgpu-core/src/command/render.rs | 12 +++--- wgpu-core/src/device/queue.rs | 4 +- wgpu-core/src/device/resource.rs | 15 +++++++- wgpu-core/src/track/buffer.rs | 36 +++++------------- wgpu-core/src/track/metadata.rs | 16 +++----- wgpu-core/src/track/mod.rs | 60 ++++++++++++++++++++++-------- wgpu-core/src/track/pool.rs | 63 -------------------------------- wgpu-core/src/track/texture.rs | 40 ++++++-------------- 9 files changed, 98 insertions(+), 156 deletions(-) delete mode 100644 wgpu-core/src/track/pool.rs diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index c2fd3ab397..f41c1d5e3e 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -272,14 +272,14 @@ where } } -struct State { +struct State<'a, A: HalApi> { binder: Binder, pipeline: Option, - scope: UsageScope, + scope: UsageScope<'a, A>, debug_scope_depth: u32, } -impl State { +impl<'a, A: HalApi> State<'a, A> { fn is_ready(&self) -> Result<(), DispatchError> { let bind_mask = self.binder.invalid_mask(); if bind_mask != 0 { @@ -407,7 +407,7 @@ impl Global { let mut state = State { binder: Binder::new(), pipeline: None, - scope: UsageScope::new(&device.tracker_indices), + scope: device.get_usage_scope(), debug_scope_depth: 0, }; let mut temp_offsets = Vec::new(); diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 9141ddb021..16f59370b0 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -739,9 +739,9 @@ impl TextureView { const MAX_TOTAL_ATTACHMENTS: usize = hal::MAX_COLOR_ATTACHMENTS + hal::MAX_COLOR_ATTACHMENTS + 1; type AttachmentDataVec = ArrayVec; -struct RenderPassInfo<'a, A: HalApi> { +struct RenderPassInfo<'a, 'd, A: HalApi> { context: RenderPassContext, - usage_scope: UsageScope, + usage_scope: UsageScope<'d, A>, /// All render attachments, including depth/stencil render_attachments: AttachmentDataVec>, is_depth_read_only: bool, @@ -754,7 +754,7 @@ struct RenderPassInfo<'a, A: HalApi> { multiview: Option, } -impl<'a, A: HalApi> RenderPassInfo<'a, A> { +impl<'a, 'd, A: HalApi> RenderPassInfo<'a, 'd, A> { fn add_pass_texture_init_actions( channel: &PassChannel, texture_memory_actions: &mut CommandBufferTextureMemoryActions, @@ -790,7 +790,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { } fn start( - device: &Device, + device: &'d Device, label: Option<&str>, color_attachments: &[Option], depth_stencil_attachment: Option<&RenderPassDepthStencilAttachment>, @@ -1214,7 +1214,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { Ok(Self { context, - usage_scope: UsageScope::new(&device.tracker_indices), + usage_scope: device.get_usage_scope(), render_attachments, is_depth_read_only, is_stencil_read_only, @@ -1230,7 +1230,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { mut self, raw: &mut A::CommandEncoder, snatch_guard: &SnatchGuard, - ) -> Result<(UsageScope, SurfacesInDiscardState), RenderPassErrorInner> { + ) -> Result<(UsageScope<'d, A>, SurfacesInDiscardState), RenderPassErrorInner> { profiling::scope!("RenderPassInfo::finish"); unsafe { raw.end_render_pass(); diff --git a/wgpu-core/src/device/queue.rs b/wgpu-core/src/device/queue.rs index 6ebb9eb09b..cb558ad6f0 100644 --- a/wgpu-core/src/device/queue.rs +++ b/wgpu-core/src/device/queue.rs @@ -1155,7 +1155,7 @@ impl Global { + 1; let mut active_executions = Vec::new(); - let mut used_surface_textures = track::TextureUsageScope::new(); + let mut used_surface_textures = track::TextureUsageScope::default(); let snatch_guard = device.snatchable_lock.read(); @@ -1435,7 +1435,7 @@ impl Global { baked.encoder.end_encoding().unwrap() }; baked.list.push(present); - used_surface_textures = track::TextureUsageScope::new(); + used_surface_textures = track::TextureUsageScope::default(); } // done diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index 28ba0eafb1..fff2dafd5c 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -28,7 +28,7 @@ use crate::{ resource_log, snatch::{SnatchGuard, SnatchLock, Snatchable}, storage::Storage, - track::{BindGroupStates, TextureSelector, Tracker, TrackerIndexAllocators}, + track::{BindGroupStates, TextureSelector, Tracker, TrackerIndexAllocators, UsageScope}, validation::{ self, check_buffer_usage, check_texture_usage, validate_color_attachment_bytes_per_sample, }, @@ -135,6 +135,9 @@ pub struct Device { pub(crate) deferred_destroy: Mutex>>, #[cfg(feature = "trace")] pub(crate) trace: Mutex>, + + // usage_scope pool + pub(crate) usage_scopes: Mutex>>, } pub(crate) enum DeferredDestroy { @@ -296,6 +299,7 @@ impl Device { instance_flags, pending_writes: Mutex::new(Some(pending_writes)), deferred_destroy: Mutex::new(Vec::new()), + usage_scopes: Mutex::default(), }) } @@ -3568,6 +3572,15 @@ impl Device { let _ = texture.destroy(); } } + + pub(crate) fn get_usage_scope<'a>(&'a self) -> UsageScope<'a, A> { + let usage_scope = self.usage_scopes.lock().pop().unwrap_or_default(); + usage_scope.set_device(self) + } + + pub(crate) fn put_usage_scope(&self, usage_scope: UsageScope<'static, A>) { + self.usage_scopes.lock().push(usage_scope); + } } impl Device { diff --git a/wgpu-core/src/track/buffer.rs b/wgpu-core/src/track/buffer.rs index 4f73a7f023..6cf1fdda6f 100644 --- a/wgpu-core/src/track/buffer.rs +++ b/wgpu-core/src/track/buffer.rs @@ -7,10 +7,7 @@ use std::{borrow::Cow, marker::PhantomData, sync::Arc}; -use super::{ - pool::{BitvecPool, VecPool}, - PendingTransition, ResourceTracker, TrackerIndex, -}; +use super::{PendingTransition, ResourceTracker, TrackerIndex}; use crate::{ hal_api::HalApi, id::BufferId, @@ -107,44 +104,31 @@ impl BufferBindGroupState { } } -static STATE_POOL: VecPool = VecPool::new(); -static RES_POOL: VecPool = VecPool::new(); -static RES_BIT_POOL: BitvecPool = BitvecPool::new(); - /// Stores all buffer state within a single usage scope. #[derive(Debug)] pub(crate) struct BufferUsageScope { state: Vec, - metadata: ResourceMetadata>, } -impl Drop for BufferUsageScope { - fn drop(&mut self) { - let (bits, res) = self.metadata.return_vecs(); - RES_BIT_POOL.put(bits); - unsafe { - STATE_POOL.put(&mut self.state); - RES_POOL.put(res); +impl Default for BufferUsageScope { + fn default() -> Self { + Self { + state: Vec::new(), + metadata: ResourceMetadata::new(), } } } impl BufferUsageScope { - pub fn new() -> Self { - Self { - // safety: not safe if we have multiple HalApis in a single execution - state: unsafe { STATE_POOL.get() }, - metadata: ResourceMetadata::new_with_vecs(RES_BIT_POOL.get(), unsafe { - RES_POOL.get() - }), - } - } - fn tracker_assert_in_bounds(&self, index: usize) { strict_assert!(index < self.state.len()); self.metadata.tracker_assert_in_bounds(index); } + pub fn clear(&mut self) { + self.state.clear(); + self.metadata.clear(); + } /// Sets the size of all the vectors inside the tracker. /// diff --git a/wgpu-core/src/track/metadata.rs b/wgpu-core/src/track/metadata.rs index d8d3d3958a..3e71e0e084 100644 --- a/wgpu-core/src/track/metadata.rs +++ b/wgpu-core/src/track/metadata.rs @@ -29,17 +29,6 @@ impl ResourceMetadata { } } - pub(super) fn new_with_vecs(bitvec: BitVec, vec: Vec>>) -> Self { - Self { - owned: bitvec, - resources: vec, - } - } - - pub(super) fn return_vecs(&mut self) -> (&mut BitVec, &mut Vec>>) { - (&mut self.owned, &mut self.resources) - } - /// Returns the number of indices we can accommodate. pub(super) fn size(&self) -> usize { self.owned.len() @@ -50,6 +39,11 @@ impl ResourceMetadata { resize_bitvec(&mut self.owned, size); } + pub(super) fn clear(&mut self) { + self.resources.clear(); + self.owned.clear(); + } + /// Ensures a given index is in bounds for all arrays and does /// sanity checks of the presence of a refcount. /// diff --git a/wgpu-core/src/track/mod.rs b/wgpu-core/src/track/mod.rs index e2216d6283..7477163d5b 100644 --- a/wgpu-core/src/track/mod.rs +++ b/wgpu-core/src/track/mod.rs @@ -97,13 +97,13 @@ Device <- CommandBuffer = insert(device.start, device.end, buffer.start, buffer. mod buffer; mod metadata; -pub mod pool; mod range; mod stateless; mod texture; use crate::{ - binding_model, command, conv, hal_api::HalApi, id, pipeline, resource, snatch::SnatchGuard, + binding_model, command, conv, device::Device, hal_api::HalApi, id, pipeline, resource, + snatch::SnatchGuard, }; use parking_lot::{Mutex, RwLock}; @@ -481,8 +481,8 @@ impl RenderBundleScope { /// Create the render bundle scope and pull the maximum IDs from the hubs. pub fn new() -> Self { Self { - buffers: RwLock::new(BufferUsageScope::new()), - textures: RwLock::new(TextureUsageScope::new()), + buffers: RwLock::new(BufferUsageScope::default()), + textures: RwLock::new(TextureUsageScope::default()), bind_groups: RwLock::new(StatelessTracker::new()), render_pipelines: RwLock::new(StatelessTracker::new()), query_sets: RwLock::new(StatelessTracker::new()), @@ -516,25 +516,55 @@ impl RenderBundleScope { /// A usage scope tracker. Only needs to store stateful resources as stateless /// resources cannot possibly have a usage conflict. #[derive(Debug)] -pub(crate) struct UsageScope { +pub(crate) struct UsageScope<'a, A: HalApi> { + pub device: Option<&'a Device>, pub buffers: BufferUsageScope, pub textures: TextureUsageScope, } -impl UsageScope { - /// Create the render bundle scope and pull the maximum IDs from the hubs. - pub fn new(tracker_indices: &TrackerIndexAllocators) -> Self { - let mut value = Self { - buffers: BufferUsageScope::new(), - textures: TextureUsageScope::new(), - }; +impl<'a, A: HalApi> Drop for UsageScope<'a, A> { + fn drop(&mut self) { + if let Some(device) = self.device.take() { + self.buffers.clear(); + self.textures.clear(); + device.put_usage_scope(UsageScope::<'static, A> { + device: None, + buffers: std::mem::take(&mut self.buffers), + textures: std::mem::take(&mut self.textures), + }); + } + } +} + +impl Default for UsageScope<'static, A> { + fn default() -> Self { + Self { + device: Default::default(), + buffers: Default::default(), + textures: Default::default(), + } + } +} - value.buffers.set_size(tracker_indices.buffers.size()); - value.textures.set_size(tracker_indices.textures.size()); +impl UsageScope<'static, A> { + pub fn set_device<'d>(mut self, device: &'d Device) -> UsageScope<'d, A> { + let mut scope = UsageScope::<'d, A> { + device: Some(device), + buffers: std::mem::take(&mut self.buffers), + textures: std::mem::take(&mut self.textures), + }; - value + scope + .buffers + .set_size(device.tracker_indices.buffers.size()); + scope + .textures + .set_size(device.tracker_indices.textures.size()); + scope } +} +impl<'a, A: HalApi> UsageScope<'a, A> { /// Merge the inner contents of a bind group into the usage scope. /// /// Only stateful things are merged in here, all other resources are owned diff --git a/wgpu-core/src/track/pool.rs b/wgpu-core/src/track/pool.rs deleted file mode 100644 index c9ba16666a..0000000000 --- a/wgpu-core/src/track/pool.rs +++ /dev/null @@ -1,63 +0,0 @@ -use bit_vec::BitVec; -use once_cell::sync::OnceCell; -use parking_lot::Mutex; -pub struct VecPool { - pool: OnceCell>>>, -} - -impl VecPool { - pub const fn new() -> Self { - Self { - pool: OnceCell::new(), - } - } - - /// safety: a pool instance must only ever be called with a single type T to ensure transmute is sound - /// (`#[repr(rust)]` does not guarantee equal member order for different monomorphs) - pub unsafe fn get(&self) -> Vec { - unsafe { - std::mem::transmute( - self.pool - .get_or_init(Default::default) - .lock() - .pop() - .unwrap_or_default(), - ) - } - } - - /// safety: a pool instance must only ever be called with a single type T to ensure transmute is sound - /// (`#[repr(rust)]` does not guarantee equal member order for different monomorphs) - pub unsafe fn put(&self, vec: &mut Vec) { - let mut vec = std::mem::take(vec); - vec.clear(); - let vec = unsafe { std::mem::transmute(vec) }; - self.pool.get_or_init(Default::default).lock().push(vec); - } -} - -pub struct BitvecPool { - pool: OnceCell>>>, -} - -impl BitvecPool { - pub const fn new() -> Self { - Self { - pool: OnceCell::new(), - } - } - - pub fn get(&self) -> BitVec { - self.pool - .get_or_init(Default::default) - .lock() - .pop() - .unwrap_or_default() - } - - pub fn put(&self, vec: &mut BitVec) { - let mut vec = std::mem::take(vec); - vec.clear(); - self.pool.get_or_init(Default::default).lock().push(vec); - } -} diff --git a/wgpu-core/src/track/texture.rs b/wgpu-core/src/track/texture.rs index c447e9bd22..3cf95ff38a 100644 --- a/wgpu-core/src/track/texture.rs +++ b/wgpu-core/src/track/texture.rs @@ -20,9 +20,7 @@ !*/ use super::{ - pool::{BitvecPool, VecPool}, - range::RangedStates, - PendingTransition, PendingTransitionList, ResourceTracker, TrackerIndex, + range::RangedStates, PendingTransition, PendingTransitionList, ResourceTracker, TrackerIndex, }; use crate::{ hal_api::HalApi, @@ -206,8 +204,6 @@ impl TextureBindGroupState { } } -static USES_POOL: VecPool = VecPool::new(); - /// Container for corresponding simple and complex texture states. #[derive(Debug)] pub(crate) struct TextureStateSet { @@ -215,16 +211,10 @@ pub(crate) struct TextureStateSet { complex: FastHashMap, } -impl Drop for TextureStateSet { - fn drop(&mut self) { - unsafe { USES_POOL.put(&mut self.simple) } - } -} - impl TextureStateSet { fn new() -> Self { Self { - simple: unsafe { USES_POOL.get() }, + simple: Vec::new(), complex: FastHashMap::default(), } } @@ -246,27 +236,16 @@ pub(crate) struct TextureUsageScope { metadata: ResourceMetadata>, } -static RES_POOL: VecPool = VecPool::new(); -static RES_BIT_POOL: BitvecPool = BitvecPool::new(); - -impl Drop for TextureUsageScope { - fn drop(&mut self) { - let (bits, res) = self.metadata.return_vecs(); - unsafe { RES_POOL.put(res) }; - RES_BIT_POOL.put(bits); - } -} - -impl TextureUsageScope { - pub fn new() -> Self { +impl Default for TextureUsageScope { + fn default() -> Self { Self { set: TextureStateSet::new(), - metadata: ResourceMetadata::new_with_vecs(RES_BIT_POOL.get(), unsafe { - RES_POOL.get() - }), + metadata: ResourceMetadata::new(), } } +} +impl TextureUsageScope { fn tracker_assert_in_bounds(&self, index: usize) { self.metadata.tracker_assert_in_bounds(index); @@ -281,6 +260,11 @@ impl TextureUsageScope { }); } + pub fn clear(&mut self) { + self.set.clear(); + self.metadata.clear(); + } + /// Sets the size of all the vectors inside the tracker. /// /// Must be called with the highest possible Texture ID before From 0a241cc2e5a6b2969449bb480ac0bcf78e65f4e4 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Thu, 21 Mar 2024 10:05:12 +0000 Subject: [PATCH 5/8] use pool ref, cleanup and comment --- wgpu-core/src/device/resource.rs | 16 ++++----- wgpu-core/src/track/mod.rs | 57 ++++++++++++++------------------ 2 files changed, 32 insertions(+), 41 deletions(-) diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index fff2dafd5c..f49577b0f3 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -28,7 +28,10 @@ use crate::{ resource_log, snatch::{SnatchGuard, SnatchLock, Snatchable}, storage::Storage, - track::{BindGroupStates, TextureSelector, Tracker, TrackerIndexAllocators, UsageScope}, + track::{ + BindGroupStates, TextureSelector, Tracker, TrackerIndexAllocators, UsageScope, + UsageScopePool, + }, validation::{ self, check_buffer_usage, check_texture_usage, validate_color_attachment_bytes_per_sample, }, @@ -137,7 +140,7 @@ pub struct Device { pub(crate) trace: Mutex>, // usage_scope pool - pub(crate) usage_scopes: Mutex>>, + pub(crate) usage_scopes: UsageScopePool, } pub(crate) enum DeferredDestroy { @@ -299,7 +302,7 @@ impl Device { instance_flags, pending_writes: Mutex::new(Some(pending_writes)), deferred_destroy: Mutex::new(Vec::new()), - usage_scopes: Mutex::default(), + usage_scopes: Default::default(), }) } @@ -3574,12 +3577,7 @@ impl Device { } pub(crate) fn get_usage_scope<'a>(&'a self) -> UsageScope<'a, A> { - let usage_scope = self.usage_scopes.lock().pop().unwrap_or_default(); - usage_scope.set_device(self) - } - - pub(crate) fn put_usage_scope(&self, usage_scope: UsageScope<'static, A>) { - self.usage_scopes.lock().push(usage_scope); + UsageScope::new_pooled(&self.usage_scopes, &self.tracker_indices) } } diff --git a/wgpu-core/src/track/mod.rs b/wgpu-core/src/track/mod.rs index 7477163d5b..5eb4a9804c 100644 --- a/wgpu-core/src/track/mod.rs +++ b/wgpu-core/src/track/mod.rs @@ -102,8 +102,7 @@ mod stateless; mod texture; use crate::{ - binding_model, command, conv, device::Device, hal_api::HalApi, id, pipeline, resource, - snatch::SnatchGuard, + binding_model, command, conv, hal_api::HalApi, id, pipeline, resource, snatch::SnatchGuard, }; use parking_lot::{Mutex, RwLock}; @@ -513,53 +512,47 @@ impl RenderBundleScope { } } +/// A pool for storing the memory used by [`UsageScope`]s and [`RenderBundleScope`]s. We take and store this memory when the +/// scope is dropped to avoid reallocating. The memory required only grows and allocation cost is +/// significant when a large number of resources have been used. +pub(crate) type UsageScopePool = Mutex, TextureUsageScope)>>; + /// A usage scope tracker. Only needs to store stateful resources as stateless /// resources cannot possibly have a usage conflict. #[derive(Debug)] pub(crate) struct UsageScope<'a, A: HalApi> { - pub device: Option<&'a Device>, + pub pool: &'a UsageScopePool, pub buffers: BufferUsageScope, pub textures: TextureUsageScope, } impl<'a, A: HalApi> Drop for UsageScope<'a, A> { fn drop(&mut self) { - if let Some(device) = self.device.take() { - self.buffers.clear(); - self.textures.clear(); - device.put_usage_scope(UsageScope::<'static, A> { - device: None, - buffers: std::mem::take(&mut self.buffers), - textures: std::mem::take(&mut self.textures), - }); - } - } -} - -impl Default for UsageScope<'static, A> { - fn default() -> Self { - Self { - device: Default::default(), - buffers: Default::default(), - textures: Default::default(), - } + // clear vecs and push into pool + self.buffers.clear(); + self.textures.clear(); + self.pool.lock().push(( + std::mem::take(&mut self.buffers), + std::mem::take(&mut self.textures), + )); } } impl UsageScope<'static, A> { - pub fn set_device<'d>(mut self, device: &'d Device) -> UsageScope<'d, A> { + pub fn new_pooled<'d>( + pool: &'d UsageScopePool, + tracker_indices: &TrackerIndexAllocators, + ) -> UsageScope<'d, A> { + let pooled = pool.lock().pop().unwrap_or_default(); + let mut scope = UsageScope::<'d, A> { - device: Some(device), - buffers: std::mem::take(&mut self.buffers), - textures: std::mem::take(&mut self.textures), + pool, + buffers: pooled.0, + textures: pooled.1, }; - scope - .buffers - .set_size(device.tracker_indices.buffers.size()); - scope - .textures - .set_size(device.tracker_indices.textures.size()); + scope.buffers.set_size(tracker_indices.buffers.size()); + scope.textures.set_size(tracker_indices.textures.size()); scope } } From 86d134397e64bd39a6129f009c7187520e68439d Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Mon, 18 Mar 2024 14:53:41 +0000 Subject: [PATCH 6/8] suspect all the future suspects (#5413) * suspect all the future suspects * changelog --- CHANGELOG.md | 1 + wgpu-core/src/device/life.rs | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0fa55fafc..3bd720e796 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -132,6 +132,7 @@ Bottom level categories: - Fix registry leaks with de-duplicated resources. By @nical in [#5244](https://github.com/gfx-rs/wgpu/pull/5244) - Fix behavior of integer `clamp` when `min` argument > `max` argument. By @cwfitzgerald in [#5300](https://github.com/gfx-rs/wgpu/pull/5300). - Fix linking when targeting android. By @ashdnazg in [#5326](https://github.com/gfx-rs/wgpu/pull/5326). +- fix resource leak for buffers/textures dropped while having pending writes. By @robtfm in [#5413](https://github.com/gfx-rs/wgpu/pull/5413) #### Naga - In spv-in, remove unnecessary "gl_PerVertex" name check so unused builtins will always be skipped. By @Imberflur in [#5227](https://github.com/gfx-rs/wgpu/pull/5227). diff --git a/wgpu-core/src/device/life.rs b/wgpu-core/src/device/life.rs index 7b06a4a30b..7db464c069 100644 --- a/wgpu-core/src/device/life.rs +++ b/wgpu-core/src/device/life.rs @@ -309,12 +309,12 @@ impl LifetimeTracker { } pub fn post_submit(&mut self) { - for v in self.future_suspected_buffers.drain(..).take(1) { + for v in self.future_suspected_buffers.drain(..) { self.suspected_resources .buffers .insert(v.as_info().tracker_index(), v); } - for v in self.future_suspected_textures.drain(..).take(1) { + for v in self.future_suspected_textures.drain(..) { self.suspected_resources .textures .insert(v.as_info().tracker_index(), v); From 0aab3c7f0a6bc51e4273dbc96bfb3be237e14070 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Thu, 21 Mar 2024 10:34:01 +0000 Subject: [PATCH 7/8] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bd720e796..6d1c472379 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -133,6 +133,7 @@ Bottom level categories: - Fix behavior of integer `clamp` when `min` argument > `max` argument. By @cwfitzgerald in [#5300](https://github.com/gfx-rs/wgpu/pull/5300). - Fix linking when targeting android. By @ashdnazg in [#5326](https://github.com/gfx-rs/wgpu/pull/5326). - fix resource leak for buffers/textures dropped while having pending writes. By @robtfm in [#5413](https://github.com/gfx-rs/wgpu/pull/5413) +- use memory pooling for UsageScopes to avoid frequent large allocations. by @robtfm in [#5414](https://github.com/gfx-rs/wgpu/pull/5414) #### Naga - In spv-in, remove unnecessary "gl_PerVertex" name check so unused builtins will always be skipped. By @Imberflur in [#5227](https://github.com/gfx-rs/wgpu/pull/5227). From 753737e185ea1221889a1c5ef7ecd524fd48c165 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Sat, 23 Mar 2024 10:56:37 +0000 Subject: [PATCH 8/8] review feedback --- wgpu-core/src/command/compute.rs | 2 +- wgpu-core/src/command/render.rs | 2 +- wgpu-core/src/device/resource.rs | 4 +--- wgpu-core/src/track/mod.rs | 2 +- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index f41c1d5e3e..941197b33a 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -407,7 +407,7 @@ impl Global { let mut state = State { binder: Binder::new(), pipeline: None, - scope: device.get_usage_scope(), + scope: device.new_usage_scope(), debug_scope_depth: 0, }; let mut temp_offsets = Vec::new(); diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 16f59370b0..8b22b4275a 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1214,7 +1214,7 @@ impl<'a, 'd, A: HalApi> RenderPassInfo<'a, 'd, A> { Ok(Self { context, - usage_scope: device.get_usage_scope(), + usage_scope: device.new_usage_scope(), render_attachments, is_depth_read_only, is_stencil_read_only, diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index f49577b0f3..04ca94ca96 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -138,8 +138,6 @@ pub struct Device { pub(crate) deferred_destroy: Mutex>>, #[cfg(feature = "trace")] pub(crate) trace: Mutex>, - - // usage_scope pool pub(crate) usage_scopes: UsageScopePool, } @@ -3576,7 +3574,7 @@ impl Device { } } - pub(crate) fn get_usage_scope<'a>(&'a self) -> UsageScope<'a, A> { + pub(crate) fn new_usage_scope(&self) -> UsageScope<'_, A> { UsageScope::new_pooled(&self.usage_scopes, &self.tracker_indices) } } diff --git a/wgpu-core/src/track/mod.rs b/wgpu-core/src/track/mod.rs index 5eb4a9804c..374dfe7493 100644 --- a/wgpu-core/src/track/mod.rs +++ b/wgpu-core/src/track/mod.rs @@ -512,7 +512,7 @@ impl RenderBundleScope { } } -/// A pool for storing the memory used by [`UsageScope`]s and [`RenderBundleScope`]s. We take and store this memory when the +/// A pool for storing the memory used by [`UsageScope`]s. We take and store this memory when the /// scope is dropped to avoid reallocating. The memory required only grows and allocation cost is /// significant when a large number of resources have been used. pub(crate) type UsageScopePool = Mutex, TextureUsageScope)>>;