From aade481bdf7f8f9ae18423bf9f0dc1279844f37e Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Tue, 19 Dec 2023 10:29:32 +0100 Subject: [PATCH] Remove some locks in BindGroup (#4894) * Remove some locks in BindGroup These are only written to clear the vectors when triaging bindgroups for destruction, which is not necessary. We can let the reference counts drop when the bind group is dropped. * Make the mem_leak test pass again --- tests/tests/mem_leaks.rs | 4 ++++ wgpu-core/src/binding_model.rs | 12 +++++------- wgpu-core/src/command/bundle.rs | 10 +++++----- wgpu-core/src/command/compute.rs | 20 ++++++++------------ wgpu-core/src/command/render.rs | 20 ++++++++------------ wgpu-core/src/device/life.rs | 4 ---- wgpu-core/src/device/resource.rs | 6 +++--- 7 files changed, 33 insertions(+), 43 deletions(-) diff --git a/tests/tests/mem_leaks.rs b/tests/tests/mem_leaks.rs index 52082e99f7..f8dc4ab9d4 100644 --- a/tests/tests/mem_leaks.rs +++ b/tests/tests/mem_leaks.rs @@ -243,6 +243,10 @@ fn draw_test_with_reports( ctx.device .poll(wgpu::Maintain::WaitForSubmissionIndex(submit_index)); + // Because of dependency between resources, A resource being destroyed during poll + // can cause another resource to be ready for destruction next time poll is called. + // Call poll twice to ensure all destroyable resources are destroyed. + ctx.device.poll(wgpu::Maintain::Poll); let global_report = ctx.instance.generate_report(); let report = global_report.hub_report(ctx.adapter_info.backend); diff --git a/wgpu-core/src/binding_model.rs b/wgpu-core/src/binding_model.rs index 80f6b9da47..2dee28f6e9 100644 --- a/wgpu-core/src/binding_model.rs +++ b/wgpu-core/src/binding_model.rs @@ -16,7 +16,6 @@ use crate::{ use arrayvec::ArrayVec; -use parking_lot::RwLock; #[cfg(feature = "replay")] use serde::Deserialize; #[cfg(feature = "trace")] @@ -815,9 +814,9 @@ pub struct BindGroup { pub(crate) layout: Arc>, pub(crate) info: ResourceInfo, pub(crate) used: BindGroupStates, - pub(crate) used_buffer_ranges: RwLock>>, - pub(crate) used_texture_ranges: RwLock>>, - pub(crate) dynamic_binding_info: RwLock>, + pub(crate) used_buffer_ranges: Vec>, + pub(crate) used_texture_ranges: Vec>, + pub(crate) dynamic_binding_info: Vec, /// Actual binding sizes for buffers that don't have `min_binding_size` /// specified in BGL. Listed in the order of iteration of `BGL.entries`. pub(crate) late_buffer_binding_sizes: Vec, @@ -845,17 +844,16 @@ impl BindGroup { offsets: &[wgt::DynamicOffset], limits: &wgt::Limits, ) -> Result<(), BindError> { - if self.dynamic_binding_info.read().len() != offsets.len() { + if self.dynamic_binding_info.len() != offsets.len() { return Err(BindError::MismatchedDynamicOffsetCount { group: bind_group_index, - expected: self.dynamic_binding_info.read().len(), + expected: self.dynamic_binding_info.len(), actual: offsets.len(), }); } for (idx, (info, &offset)) in self .dynamic_binding_info - .read() .iter() .zip(offsets.iter()) .enumerate() diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index 653ed91303..2f1cc1c8a5 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -318,10 +318,10 @@ impl RenderBundleEncoder { next_dynamic_offset = offsets_range.end; let offsets = &base.dynamic_offsets[offsets_range.clone()]; - if bind_group.dynamic_binding_info.read().len() != offsets.len() { + if bind_group.dynamic_binding_info.len() != offsets.len() { return Err(RenderCommandError::InvalidDynamicOffsetCount { actual: offsets.len(), - expected: bind_group.dynamic_binding_info.read().len(), + expected: bind_group.dynamic_binding_info.len(), }) .map_pass_err(scope); } @@ -330,7 +330,7 @@ impl RenderBundleEncoder { for (offset, info) in offsets .iter() .map(|offset| *offset as wgt::BufferAddress) - .zip(bind_group.dynamic_binding_info.read().iter()) + .zip(bind_group.dynamic_binding_info.iter()) { let (alignment, limit_name) = buffer_binding_type_alignment(&device.limits, info.binding_type); @@ -342,8 +342,8 @@ impl RenderBundleEncoder { } } - buffer_memory_init_actions.extend_from_slice(&bind_group.used_buffer_ranges.read()); - texture_memory_init_actions.extend_from_slice(&bind_group.used_texture_ranges.read()); + buffer_memory_init_actions.extend_from_slice(&bind_group.used_buffer_ranges); + texture_memory_init_actions.extend_from_slice(&bind_group.used_texture_ranges); state.set_bind_group(index, bind_group_guard.get(bind_group_id).as_ref().unwrap(), &bind_group.layout, offsets_range); unsafe { diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 0fd80f2f21..23f5b816cf 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -527,20 +527,16 @@ impl Global { .map_pass_err(scope)?; buffer_memory_init_actions.extend( - bind_group - .used_buffer_ranges - .read() - .iter() - .filter_map(|action| { - action - .buffer - .initialization_status - .read() - .check_action(action) - }), + bind_group.used_buffer_ranges.iter().filter_map(|action| { + action + .buffer + .initialization_status + .read() + .check_action(action) + }), ); - for action in bind_group.used_texture_ranges.read().iter() { + for action in bind_group.used_texture_ranges.iter() { pending_discard_init_fixups .extend(texture_memory_actions.register_init_action(action)); } diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 2e902ecf2f..0e63a10fb3 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1462,19 +1462,15 @@ impl Global { // is held to the bind group itself. buffer_memory_init_actions.extend( - bind_group - .used_buffer_ranges - .read() - .iter() - .filter_map(|action| { - action - .buffer - .initialization_status - .read() - .check_action(action) - }), + bind_group.used_buffer_ranges.iter().filter_map(|action| { + action + .buffer + .initialization_status + .read() + .check_action(action) + }), ); - for action in bind_group.used_texture_ranges.read().iter() { + for action in bind_group.used_texture_ranges.iter() { info.pending_discard_init_fixups .extend(texture_memory_actions.register_init_action(action)); } diff --git a/wgpu-core/src/device/life.rs b/wgpu-core/src/device/life.rs index 1daba3a6b8..ce6edadb4c 100644 --- a/wgpu-core/src/device/life.rs +++ b/wgpu-core/src/device/life.rs @@ -524,10 +524,6 @@ impl LifetimeTracker { .samplers .insert(v.as_info().id(), v); } - //Releasing safely unused resources to decrement refcount - bind_group.used_buffer_ranges.write().clear(); - bind_group.used_texture_ranges.write().clear(); - bind_group.dynamic_binding_info.write().clear(); self.suspected_resources .bind_group_layouts diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index 6874cdbcbc..cee8d6e949 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -2196,9 +2196,9 @@ impl Device { layout: layout.clone(), info: ResourceInfo::new(desc.label.borrow_or_default()), used, - used_buffer_ranges: RwLock::new(used_buffer_ranges), - used_texture_ranges: RwLock::new(used_texture_ranges), - dynamic_binding_info: RwLock::new(dynamic_binding_info), + used_buffer_ranges, + used_texture_ranges, + dynamic_binding_info, // collect in the order of BGL iteration late_buffer_binding_sizes: layout .entries