Skip to content

Commit

Permalink
Remove some locks in BindGroup (#4894)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
nical authored Dec 19, 2023
1 parent a1b183f commit aade481
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 43 deletions.
4 changes: 4 additions & 0 deletions tests/tests/mem_leaks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 5 additions & 7 deletions wgpu-core/src/binding_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use crate::{

use arrayvec::ArrayVec;

use parking_lot::RwLock;
#[cfg(feature = "replay")]
use serde::Deserialize;
#[cfg(feature = "trace")]
Expand Down Expand Up @@ -815,9 +814,9 @@ pub struct BindGroup<A: HalApi> {
pub(crate) layout: Arc<BindGroupLayout<A>>,
pub(crate) info: ResourceInfo<BindGroupId>,
pub(crate) used: BindGroupStates<A>,
pub(crate) used_buffer_ranges: RwLock<Vec<BufferInitTrackerAction<A>>>,
pub(crate) used_texture_ranges: RwLock<Vec<TextureInitTrackerAction<A>>>,
pub(crate) dynamic_binding_info: RwLock<Vec<BindGroupDynamicBindingData>>,
pub(crate) used_buffer_ranges: Vec<BufferInitTrackerAction<A>>,
pub(crate) used_texture_ranges: Vec<TextureInitTrackerAction<A>>,
pub(crate) dynamic_binding_info: Vec<BindGroupDynamicBindingData>,
/// 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<wgt::BufferSize>,
Expand Down Expand Up @@ -845,17 +844,16 @@ impl<A: HalApi> BindGroup<A> {
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()
Expand Down
10 changes: 5 additions & 5 deletions wgpu-core/src/command/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
Expand All @@ -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 {
Expand Down
20 changes: 8 additions & 12 deletions wgpu-core/src/command/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,20 +527,16 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.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));
}
Expand Down
20 changes: 8 additions & 12 deletions wgpu-core/src/command/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1462,19 +1462,15 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
// 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));
}
Expand Down
4 changes: 0 additions & 4 deletions wgpu-core/src/device/life.rs
Original file line number Diff line number Diff line change
Expand Up @@ -524,10 +524,6 @@ impl<A: HalApi> LifetimeTracker<A> {
.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
Expand Down
6 changes: 3 additions & 3 deletions wgpu-core/src/device/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2196,9 +2196,9 @@ impl<A: HalApi> Device<A> {
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
Expand Down

0 comments on commit aade481

Please sign in to comment.