Skip to content

Commit

Permalink
Releasing bind group resources fix missing leaks
Browse files Browse the repository at this point in the history
  • Loading branch information
@gents83
gents83 committed Sep 23, 2023
1 parent 0eac205 commit 3530dcb
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 41 deletions.
12 changes: 7 additions & 5 deletions wgpu-core/src/binding_model.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use crate::{

use arrayvec::ArrayVec;

use parking_lot::RwLock;
#[cfg(feature = "replay")]
use serde::Deserialize;
#[cfg(feature = "trace")]
Expand Down Expand Up @@ -847,9 +848,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: Vec<BufferInitTrackerAction<A>>,
pub(crate) used_texture_ranges: Vec<TextureInitTrackerAction<A>>,
pub(crate) dynamic_binding_info: Vec<BindGroupDynamicBindingData>,
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>>,
/// 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 @@ -877,16 +878,17 @@ impl<A: HalApi> BindGroup<A> {
offsets: &[wgt::DynamicOffset],
limits: &wgt::Limits,
) -> Result<(), BindError> {
if self.dynamic_binding_info.len() != offsets.len() {
if self.dynamic_binding_info.read().len() != offsets.len() {
return Err(BindError::MismatchedDynamicOffsetCount {
group: bind_group_index,
expected: self.dynamic_binding_info.len(),
expected: self.dynamic_binding_info.read().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
View file
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.len() != offsets.len() {
if bind_group.dynamic_binding_info.read().len() != offsets.len() {
return Err(RenderCommandError::InvalidDynamicOffsetCount {
actual: offsets.len(),
expected: bind_group.dynamic_binding_info.len(),
expected: bind_group.dynamic_binding_info.read().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.iter())
.zip(bind_group.dynamic_binding_info.read().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);
texture_memory_init_actions.extend_from_slice(&bind_group.used_texture_ranges);
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());

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: 12 additions & 8 deletions wgpu-core/src/command/compute.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -505,16 +505,20 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.map_pass_err(scope)?;

buffer_memory_init_actions.extend(
bind_group.used_buffer_ranges.iter().filter_map(|action| {
action
.buffer
.initialization_status
.read()
.check_action(action)
}),
bind_group
.used_buffer_ranges
.read()
.iter()
.filter_map(|action| {
action
.buffer
.initialization_status
.read()
.check_action(action)
}),
);

for action in bind_group.used_texture_ranges.iter() {
for action in bind_group.used_texture_ranges.read().iter() {
pending_discard_init_fixups
.extend(texture_memory_actions.register_init_action(action));
}
Expand Down
20 changes: 12 additions & 8 deletions wgpu-core/src/command/render.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1424,15 +1424,19 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
// is held to the bind group itself.

buffer_memory_init_actions.extend(
bind_group.used_buffer_ranges.iter().filter_map(|action| {
action
.buffer
.initialization_status
.read()
.check_action(action)
}),
bind_group
.used_buffer_ranges
.read()
.iter()
.filter_map(|action| {
action
.buffer
.initialization_status
.read()
.check_action(action)
}),
);
for action in bind_group.used_texture_ranges.iter() {
for action in bind_group.used_texture_ranges.read().iter() {
info.pending_discard_init_fixups
.extend(texture_memory_actions.register_init_action(action));
}
Expand Down
53 changes: 41 additions & 12 deletions wgpu-core/src/device/life.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,33 +418,37 @@ impl<A: HalApi> LifetimeTracker<A> {
}

impl<A: HalApi> LifetimeTracker<A> {
fn triage_resources<Id, R, F>(
fn triage_resources<Id, R, F, T>(
resources_map: &mut HashMap<Id, Arc<R>>,
active: &mut [ActiveSubmission<A>],
free_resources: &mut ResourceMaps,
trackers: &mut impl ResourceTracker<Id, R>,
registry: &Registry<Id, R>,
mut f: F,
count_fn: F,
mut on_remove: T,
) -> Vec<Arc<R>>
where
Id: id::TypedId,
R: Resource<Id>,
F: FnMut(&Id, &Arc<R>),
F: Fn(u64, &[ActiveSubmission<A>], &Id) -> usize,
T: FnMut(&Id, &Arc<R>),
{
let mut removed_resources = Vec::new();
resources_map.retain(|&id, resource| {
let submit_index = resource.as_info().submission_index();
let mut count = 1;
count += count_fn(submit_index, active, &id);
count += registry.contains(id) as usize;

let non_referenced_resources = active
.iter_mut()
.find(|a| a.index == submit_index)
.map_or(&mut *free_resources, |a| &mut a.last_resources);

let mut count = 1;
count += registry.contains(id) as usize;
count += non_referenced_resources.contains::<Id, R>(&id) as usize;

let is_removed = trackers.remove_abandoned(id, count);
if is_removed {
f(&id, resource);
on_remove(&id, resource);
removed_resources.push(resource.clone());
non_referenced_resources.insert(id, resource.clone());
}
Expand All @@ -467,6 +471,7 @@ impl<A: HalApi> LifetimeTracker<A> {
&mut self.free_resources,
&mut trackers.bundles,
&hub.render_bundles,
|_submit_index, _active, _id| 0,
|_bundle_id, _bundle| {
#[cfg(feature = "trace")]
if let Some(ref mut t) = *trace {
Expand All @@ -476,19 +481,19 @@ impl<A: HalApi> LifetimeTracker<A> {
);
removed_resources.drain(..).for_each(|bundle| {
for v in bundle.used.buffers.write().drain_resources() {
self.suspected_resources.insert(v.as_info().id(), v.clone());
self.suspected_resources.insert(v.as_info().id(), v);
}
for v in bundle.used.textures.write().drain_resources() {
self.suspected_resources.insert(v.as_info().id(), v.clone());
self.suspected_resources.insert(v.as_info().id(), v);
}
for v in bundle.used.bind_groups.write().drain_resources() {
self.suspected_resources.insert(v.as_info().id(), v.clone());
self.suspected_resources.insert(v.as_info().id(), v);
}
for v in bundle.used.render_pipelines.write().drain_resources() {
self.suspected_resources.insert(v.as_info().id(), v.clone());
self.suspected_resources.insert(v.as_info().id(), v);
}
for v in bundle.used.query_sets.write().drain_resources() {
self.suspected_resources.insert(v.as_info().id(), v.clone());
self.suspected_resources.insert(v.as_info().id(), v);
}
});
self
Expand All @@ -508,6 +513,7 @@ impl<A: HalApi> LifetimeTracker<A> {
&mut self.free_resources,
&mut trackers.bind_groups,
&hub.bind_groups,
|_submit_index, _active, _id| 0,
|_bind_group_id, _bind_group| {
#[cfg(feature = "trace")]
if let Some(ref mut t) = *trace {
Expand All @@ -528,6 +534,10 @@ impl<A: HalApi> LifetimeTracker<A> {
for v in bind_group.used.samplers.drain_resources() {
self.suspected_resources.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
.insert(bind_group.layout.as_info().id(), bind_group.layout.clone());
Expand All @@ -549,6 +559,7 @@ impl<A: HalApi> LifetimeTracker<A> {
&mut self.free_resources,
&mut trackers.views,
&hub.texture_views,
|_submit_index, _active, _id| 0,
|_texture_view_id, _texture_view| {
#[cfg(feature = "trace")]
if let Some(ref mut t) = *trace {
Expand Down Expand Up @@ -580,6 +591,7 @@ impl<A: HalApi> LifetimeTracker<A> {
&mut self.free_resources,
&mut trackers.textures,
&hub.textures,
|_submit_index, _active, _id| 0,
|_texture_id, _texture| {
#[cfg(feature = "trace")]
if let Some(ref mut t) = *trace {
Expand All @@ -604,6 +616,7 @@ impl<A: HalApi> LifetimeTracker<A> {
&mut self.free_resources,
&mut trackers.samplers,
&hub.samplers,
|_submit_index, _active, _id| 0,
|_sampler_id, _sampler| {
#[cfg(feature = "trace")]
if let Some(ref mut t) = *trace {
Expand All @@ -628,6 +641,19 @@ impl<A: HalApi> LifetimeTracker<A> {
&mut self.free_resources,
&mut trackers.buffers,
&hub.buffers,
|submit_index, active, buffer_id| {
let mut count = 0;
let mapped = active
.iter()
.find(|a| a.index == submit_index)
.map_or(&self.mapped, |a| &a.mapped);
mapped.iter().for_each(|b| {
if b.as_info().id() == *buffer_id {
count += 1;
}
});
count
},
|_buffer_id, _buffer| {
#[cfg(feature = "trace")]
if let Some(ref mut t) = *trace {
Expand Down Expand Up @@ -661,6 +687,7 @@ impl<A: HalApi> LifetimeTracker<A> {
&mut self.free_resources,
&mut trackers.compute_pipelines,
&hub.compute_pipelines,
|_submit_index, _active, _id| 0,
|_compute_pipeline_id, _compute_pipeline| {
#[cfg(feature = "trace")]
if let Some(ref mut t) = *trace {
Expand Down Expand Up @@ -691,6 +718,7 @@ impl<A: HalApi> LifetimeTracker<A> {
&mut self.free_resources,
&mut trackers.render_pipelines,
&hub.render_pipelines,
|_submit_index, _active, _id| 0,
|_render_pipeline_id, _render_pipeline| {
#[cfg(feature = "trace")]
if let Some(ref mut t) = *trace {
Expand Down Expand Up @@ -767,6 +795,7 @@ impl<A: HalApi> LifetimeTracker<A> {
&mut self.free_resources,
&mut trackers.query_sets,
&hub.query_sets,
|_submit_index, _active, _id| 0,
|_query_set_id, _query_set| {},
);
self
Expand Down
6 changes: 3 additions & 3 deletions wgpu-core/src/device/resource.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2049,9 +2049,9 @@ impl<A: HalApi> Device<A> {
layout: layout.clone(),
info: ResourceInfo::new(desc.label.borrow_or_default()),
used,
used_buffer_ranges,
used_texture_ranges,
dynamic_binding_info,
used_buffer_ranges: RwLock::new(used_buffer_ranges),
used_texture_ranges: RwLock::new(used_texture_ranges),
dynamic_binding_info: RwLock::new(dynamic_binding_info),
// collect in the order of BGL iteration
late_buffer_binding_sizes: layout
.entries
Expand Down

0 comments on commit 3530dcb

Please sign in to comment.