Remove resources ONLY when needed inside wgpu and not in user land (#4782)

This commit is contained in:
Mauro Gentile 2023-11-27 13:09:50 +01:00 committed by GitHub
parent 4f24c31765
commit 2c67f79970
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 38 additions and 126 deletions

View File

@ -13,8 +13,11 @@ static DISCARDING_COLOR_TARGET_RESETS_TEXTURE_INIT_STATE_CHECK_VISIBLE_ON_COPY_A
let mut case = TestCase::new(&mut ctx, TextureFormat::Rgba8UnormSrgb);
case.create_command_encoder();
case.discard();
case.submit_command_encoder();
case.create_command_encoder();
case.copy_texture_to_buffer();
case.submit_command_encoder_and_wait();
case.submit_command_encoder();
case.assert_buffers_are_zero();
});
@ -28,7 +31,7 @@ static DISCARDING_COLOR_TARGET_RESETS_TEXTURE_INIT_STATE_CHECK_VISIBLE_ON_COPY_I
case.create_command_encoder();
case.discard();
case.copy_texture_to_buffer();
case.submit_command_encoder_and_wait();
case.submit_command_encoder();
case.assert_buffers_are_zero();
});
@ -55,7 +58,7 @@ static DISCARDING_DEPTH_TARGET_RESETS_TEXTURE_INIT_STATE_CHECK_VISIBLE_ON_COPY_I
case.create_command_encoder();
case.discard();
case.copy_texture_to_buffer();
case.submit_command_encoder_and_wait();
case.submit_command_encoder();
case.assert_buffers_are_zero();
}
@ -70,13 +73,7 @@ static DISCARDING_EITHER_DEPTH_OR_STENCIL_ASPECT_TEST: GpuTestConfiguration =
DownlevelFlags::DEPTH_TEXTURE_AND_BUFFER_COPIES
| DownlevelFlags::COMPUTE_SHADERS,
)
.limits(Limits::downlevel_defaults())
// https://github.com/gfx-rs/wgpu/issues/4740
.expect_fail(
FailureCase::backend_adapter(Backends::VULKAN, "llvmpipe")
.panic("texture was not fully cleared")
.flaky(),
),
.limits(Limits::downlevel_defaults()),
)
.run_sync(|mut ctx| {
for format in [
@ -89,9 +86,15 @@ static DISCARDING_EITHER_DEPTH_OR_STENCIL_ASPECT_TEST: GpuTestConfiguration =
let mut case = TestCase::new(&mut ctx, format);
case.create_command_encoder();
case.discard_depth();
case.submit_command_encoder();
case.create_command_encoder();
case.discard_stencil();
case.submit_command_encoder();
case.create_command_encoder();
case.copy_texture_to_buffer();
case.submit_command_encoder_and_wait();
case.submit_command_encoder();
case.assert_buffers_are_zero();
}
@ -209,11 +212,10 @@ impl<'ctx> TestCase<'ctx> {
)
}
pub fn submit_command_encoder_and_wait(&mut self) {
pub fn submit_command_encoder(&mut self) {
self.ctx
.queue
.submit([self.encoder.take().unwrap().finish()]);
self.ctx.device.poll(MaintainBase::Wait);
}
pub fn discard(&mut self) {

View File

@ -2004,7 +2004,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
// Wait for all work to finish before configuring the surface.
let fence = device.fence.read();
let fence = fence.as_ref().unwrap();
match device.maintain(hub, fence, wgt::Maintain::Wait) {
match device.maintain(fence, wgt::Maintain::Wait) {
Ok((closures, _)) => {
user_callbacks = closures;
}
@ -2074,7 +2074,6 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
return Err(InvalidDevice);
}
device.lock_life().triage_suspected(
hub,
&device.trackers,
#[cfg(feature = "trace")]
None,
@ -2109,7 +2108,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.map_err(|_| DeviceError::Invalid)?;
let fence = device.fence.read();
let fence = fence.as_ref().unwrap();
device.maintain(hub, fence, maintain)?
device.maintain(fence, maintain)?
};
closures.fire();
@ -2143,7 +2142,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
};
let fence = device.fence.read();
let fence = fence.as_ref().unwrap();
let (cbs, queue_empty) = device.maintain(hub, fence, maintain)?;
let (cbs, queue_empty) = device.maintain(fence, maintain)?;
all_queue_empty = all_queue_empty && queue_empty;
closures.extend(cbs);

View File

@ -8,14 +8,12 @@ use crate::{
DeviceError, DeviceLostClosure,
},
hal_api::HalApi,
hub::Hub,
id::{
self, BindGroupId, BindGroupLayoutId, BufferId, ComputePipelineId, PipelineLayoutId,
QuerySetId, RenderBundleId, RenderPipelineId, SamplerId, StagingBufferId, TextureId,
TextureViewId,
},
pipeline::{ComputePipeline, RenderPipeline},
registry::Registry,
resource::{
self, Buffer, QuerySet, Resource, ResourceType, Sampler, StagingBuffer, Texture,
TextureView,
@ -77,16 +75,6 @@ impl ResourceMaps {
self.maps.insert(R::TYPE, Box::new(map));
self
}
fn map<Id, R>(&self) -> &FastHashMap<Id, Arc<R>>
where
Id: id::TypedId,
R: Resource<Id>,
{
let map = self.maps.get(R::TYPE).unwrap();
let any_map = map.as_ref().as_any();
let map = any_map.downcast_ref::<FastHashMap<Id, Arc<R>>>().unwrap();
map
}
fn map_mut<Id, R>(&mut self) -> &mut FastHashMap<Id, Arc<R>>
where
Id: id::TypedId,
@ -132,13 +120,6 @@ impl ResourceMaps {
self.map_mut().insert(id, r);
self
}
pub(crate) fn contains<Id, R>(&mut self, id: &Id) -> bool
where
Id: id::TypedId,
R: Resource<Id>,
{
self.map::<Id, R>().contains_key(id)
}
}
/// Resources used by a queue submission, and work to be done once it completes.
@ -424,35 +405,27 @@ impl<A: HalApi> LifetimeTracker<A> {
}
impl<A: HalApi> LifetimeTracker<A> {
fn triage_resources<Id, R, F, T>(
fn triage_resources<Id, R, T>(
resources_map: &mut FastHashMap<Id, Arc<R>>,
active: &mut [ActiveSubmission<A>],
free_resources: &mut ResourceMaps,
trackers: &mut impl ResourceTracker<Id, R>,
registry: &Registry<Id, R>,
count_fn: F,
mut on_remove: T,
) -> Vec<Arc<R>>
where
Id: id::TypedId,
R: Resource<Id>,
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);
count += non_referenced_resources.contains::<Id, R>(&id) as usize;
let is_removed = trackers.remove_abandoned(id, count);
let is_removed = trackers.remove_abandoned(id);
if is_removed {
on_remove(&id, resource);
removed_resources.push(resource.clone());
@ -465,7 +438,6 @@ impl<A: HalApi> LifetimeTracker<A> {
fn triage_suspected_render_bundles(
&mut self,
hub: &Hub<A>,
trackers: &Mutex<Tracker<A>>,
#[cfg(feature = "trace")] trace: &mut Option<&mut trace::Trace>,
) -> &mut Self {
@ -476,8 +448,6 @@ impl<A: HalApi> LifetimeTracker<A> {
self.active.as_mut_slice(),
&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 {
@ -507,7 +477,6 @@ impl<A: HalApi> LifetimeTracker<A> {
fn triage_suspected_bind_groups(
&mut self,
hub: &Hub<A>,
trackers: &Mutex<Tracker<A>>,
#[cfg(feature = "trace")] trace: &mut Option<&mut trace::Trace>,
) -> &mut Self {
@ -518,8 +487,6 @@ impl<A: HalApi> LifetimeTracker<A> {
self.active.as_mut_slice(),
&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 {
@ -553,7 +520,6 @@ impl<A: HalApi> LifetimeTracker<A> {
fn triage_suspected_texture_views(
&mut self,
hub: &Hub<A>,
trackers: &Mutex<Tracker<A>>,
#[cfg(feature = "trace")] trace: &mut Option<&mut trace::Trace>,
) -> &mut Self {
@ -564,8 +530,6 @@ impl<A: HalApi> LifetimeTracker<A> {
self.active.as_mut_slice(),
&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 {
@ -585,7 +549,6 @@ impl<A: HalApi> LifetimeTracker<A> {
fn triage_suspected_textures(
&mut self,
hub: &Hub<A>,
trackers: &Mutex<Tracker<A>>,
#[cfg(feature = "trace")] trace: &mut Option<&mut trace::Trace>,
) -> &mut Self {
@ -596,8 +559,6 @@ impl<A: HalApi> LifetimeTracker<A> {
self.active.as_mut_slice(),
&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 {
@ -610,7 +571,6 @@ impl<A: HalApi> LifetimeTracker<A> {
fn triage_suspected_samplers(
&mut self,
hub: &Hub<A>,
trackers: &Mutex<Tracker<A>>,
#[cfg(feature = "trace")] trace: &mut Option<&mut trace::Trace>,
) -> &mut Self {
@ -621,8 +581,6 @@ impl<A: HalApi> LifetimeTracker<A> {
self.active.as_mut_slice(),
&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 {
@ -635,7 +593,6 @@ impl<A: HalApi> LifetimeTracker<A> {
fn triage_suspected_buffers(
&mut self,
hub: &Hub<A>,
trackers: &Mutex<Tracker<A>>,
#[cfg(feature = "trace")] trace: &mut Option<&mut trace::Trace>,
) -> &mut Self {
@ -646,20 +603,6 @@ impl<A: HalApi> LifetimeTracker<A> {
self.active.as_mut_slice(),
&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 {
@ -681,7 +624,6 @@ impl<A: HalApi> LifetimeTracker<A> {
fn triage_suspected_compute_pipelines(
&mut self,
hub: &Hub<A>,
trackers: &Mutex<Tracker<A>>,
#[cfg(feature = "trace")] trace: &mut Option<&mut trace::Trace>,
) -> &mut Self {
@ -692,8 +634,6 @@ impl<A: HalApi> LifetimeTracker<A> {
self.active.as_mut_slice(),
&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 {
@ -712,7 +652,6 @@ impl<A: HalApi> LifetimeTracker<A> {
fn triage_suspected_render_pipelines(
&mut self,
hub: &Hub<A>,
trackers: &Mutex<Tracker<A>>,
#[cfg(feature = "trace")] trace: &mut Option<&mut trace::Trace>,
) -> &mut Self {
@ -723,8 +662,6 @@ impl<A: HalApi> LifetimeTracker<A> {
self.active.as_mut_slice(),
&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 {
@ -787,11 +724,7 @@ impl<A: HalApi> LifetimeTracker<A> {
self
}
fn triage_suspected_query_sets(
&mut self,
hub: &Hub<A>,
trackers: &Mutex<Tracker<A>>,
) -> &mut Self {
fn triage_suspected_query_sets(&mut self, trackers: &Mutex<Tracker<A>>) -> &mut Self {
let mut trackers = trackers.lock();
let resource_map = self.suspected_resources.map_mut();
Self::triage_resources(
@ -799,8 +732,6 @@ impl<A: HalApi> LifetimeTracker<A> {
self.active.as_mut_slice(),
&mut self.free_resources,
&mut trackers.query_sets,
&hub.query_sets,
|_submit_index, _active, _id| 0,
|_query_set_id, _query_set| {},
);
self
@ -858,7 +789,6 @@ impl<A: HalApi> LifetimeTracker<A> {
/// [`self.free_resources`]: LifetimeTracker::free_resources
pub(crate) fn triage_suspected(
&mut self,
hub: &Hub<A>,
trackers: &Mutex<Tracker<A>>,
#[cfg(feature = "trace")] mut trace: Option<&mut trace::Trace>,
) {
@ -866,25 +796,21 @@ impl<A: HalApi> LifetimeTracker<A> {
//NOTE: the order is important to release resources that depends between each other!
self.triage_suspected_render_bundles(
hub,
trackers,
#[cfg(feature = "trace")]
&mut trace,
);
self.triage_suspected_compute_pipelines(
hub,
trackers,
#[cfg(feature = "trace")]
&mut trace,
);
self.triage_suspected_render_pipelines(
hub,
trackers,
#[cfg(feature = "trace")]
&mut trace,
);
self.triage_suspected_bind_groups(
hub,
trackers,
#[cfg(feature = "trace")]
&mut trace,
@ -897,28 +823,24 @@ impl<A: HalApi> LifetimeTracker<A> {
#[cfg(feature = "trace")]
&mut trace,
);
self.triage_suspected_query_sets(hub, trackers);
self.triage_suspected_query_sets(trackers);
self.triage_suspected_samplers(
hub,
trackers,
#[cfg(feature = "trace")]
&mut trace,
);
self.triage_suspected_staging_buffers();
self.triage_suspected_texture_views(
hub,
trackers,
#[cfg(feature = "trace")]
&mut trace,
);
self.triage_suspected_textures(
hub,
trackers,
#[cfg(feature = "trace")]
&mut trace,
);
self.triage_suspected_buffers(
hub,
trackers,
#[cfg(feature = "trace")]
&mut trace,
@ -959,7 +881,6 @@ impl<A: HalApi> LifetimeTracker<A> {
#[must_use]
pub(crate) fn handle_mapping(
&mut self,
hub: &Hub<A>,
raw: &A::Device,
trackers: &Mutex<Tracker<A>>,
) -> Vec<super::BufferMapPendingClosure> {
@ -973,9 +894,7 @@ impl<A: HalApi> LifetimeTracker<A> {
let buffer_id = buffer.info.id();
let is_removed = {
let mut trackers = trackers.lock();
let mut count = 1;
count += hub.buffers.contains(buffer_id) as usize;
trackers.buffers.remove_abandoned(buffer_id, count)
trackers.buffers.remove_abandoned(buffer_id)
};
if is_removed {
*buffer.map_state.lock() = resource::BufferMapState::Idle;

View File

@ -1487,7 +1487,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
// This will schedule destruction of all resources that are no longer needed
// by the user but used in the command stream, among other things.
let (closures, _) = match device.maintain(hub, fence, wgt::Maintain::Poll) {
let (closures, _) = match device.maintain(fence, wgt::Maintain::Poll) {
Ok(closures) => closures,
Err(WaitIdleError::Device(err)) => return Err(QueueSubmitError::Queue(err)),
Err(WaitIdleError::StuckGpu) => return Err(QueueSubmitError::StuckGpu),

View File

@ -307,7 +307,6 @@ impl<A: HalApi> Device<A> {
/// return it to our callers.)
pub(crate) fn maintain<'this>(
&'this self,
hub: &Hub<A>,
fence: &A::Fence,
maintain: wgt::Maintain<queue::WrappedSubmissionIndex>,
) -> Result<(UserClosures, bool), WaitIdleError> {
@ -328,7 +327,6 @@ impl<A: HalApi> Device<A> {
life_tracker.suspected_resources.extend(temp_suspected);
life_tracker.triage_suspected(
hub,
&self.trackers,
#[cfg(feature = "trace")]
self.trace.lock().as_mut(),
@ -368,7 +366,7 @@ impl<A: HalApi> Device<A> {
last_done_index,
self.command_allocator.lock().as_mut().unwrap(),
);
let mapping_closures = life_tracker.handle_mapping(hub, self.raw(), &self.trackers);
let mapping_closures = life_tracker.handle_mapping(self.raw(), &self.trackers);
//Cleaning up resources and released all unused suspected ones
life_tracker.cleanup();

View File

@ -122,9 +122,6 @@ impl<I: id::TypedId, T: Resource<I>> Registry<I, T> {
data: &self.storage,
}
}
pub(crate) fn contains(&self, id: I) -> bool {
self.read().contains(id)
}
pub(crate) fn try_get(&self, id: I) -> Result<Option<Arc<T>>, InvalidId> {
self.read().try_get(id).map(|o| o.cloned())
}

View File

@ -313,7 +313,7 @@ impl<A: HalApi> ResourceTracker<BufferId, Buffer<A>> for BufferTracker<A> {
/// [`Device::trackers`]: crate::device::Device
/// [`self.metadata`]: BufferTracker::metadata
/// [`Hub::buffers`]: crate::hub::Hub::buffers
fn remove_abandoned(&mut self, id: BufferId, external_count: usize) -> bool {
fn remove_abandoned(&mut self, id: BufferId) -> bool {
let index = id.unzip().0 as usize;
if index > self.metadata.size() {
@ -325,10 +325,9 @@ impl<A: HalApi> ResourceTracker<BufferId, Buffer<A>> for BufferTracker<A> {
unsafe {
if self.metadata.contains_unchecked(index) {
let existing_ref_count = self.metadata.get_ref_count_unchecked(index);
//2 ref count if only in Device Tracker and suspected resource itself and already released from user
//so not appearing in Registry
let min_ref_count = 1 + external_count;
if existing_ref_count <= min_ref_count {
//RefCount 2 means that resource is hold just by DeviceTracker and this suspected resource itself
//so it's already been released from user and so it's not inside Registry\Storage
if existing_ref_count <= 2 {
self.metadata.remove(index);
log::trace!("Buffer {:?} is not tracked anymore", id,);
return true;

View File

@ -489,7 +489,7 @@ where
Id: TypedId,
R: resource::Resource<Id>,
{
fn remove_abandoned(&mut self, id: Id, external_count: usize) -> bool;
fn remove_abandoned(&mut self, id: Id) -> bool;
}
/// A full double sided tracker used by CommandBuffers and the Device.

View File

@ -85,7 +85,7 @@ impl<A: HalApi, Id: TypedId, T: Resource<Id>> ResourceTracker<Id, T>
///
/// If the ID is higher than the length of internal vectors,
/// false will be returned.
fn remove_abandoned(&mut self, id: Id, external_count: usize) -> bool {
fn remove_abandoned(&mut self, id: Id) -> bool {
let index = id.unzip().0 as usize;
if index > self.metadata.size() {
@ -99,10 +99,9 @@ impl<A: HalApi, Id: TypedId, T: Resource<Id>> ResourceTracker<Id, T>
unsafe {
if self.metadata.contains_unchecked(index) {
let existing_ref_count = self.metadata.get_ref_count_unchecked(index);
//2 ref count if only in Device Tracker and suspected resource itself and already released from user
//so not appearing in Registry
let min_ref_count = 1 + external_count;
if existing_ref_count <= min_ref_count {
//RefCount 2 means that resource is hold just by DeviceTracker and this suspected resource itself
//so it's already been released from user and so it's not inside Registry\Storage
if existing_ref_count <= 2 {
self.metadata.remove(index);
log::trace!("{} {:?} is not tracked anymore", T::TYPE, id,);
return true;

View File

@ -401,7 +401,7 @@ impl<A: HalApi> ResourceTracker<TextureId, Texture<A>> for TextureTracker<A> {
///
/// If the ID is higher than the length of internal vectors,
/// false will be returned.
fn remove_abandoned(&mut self, id: TextureId, external_count: usize) -> bool {
fn remove_abandoned(&mut self, id: TextureId) -> bool {
let index = id.unzip().0 as usize;
if index > self.metadata.size() {
@ -413,10 +413,9 @@ impl<A: HalApi> ResourceTracker<TextureId, Texture<A>> for TextureTracker<A> {
unsafe {
if self.metadata.contains_unchecked(index) {
let existing_ref_count = self.metadata.get_ref_count_unchecked(index);
//2 ref count if only in Device Tracker and suspected resource itself and already released from user
//so not appearing in Registry
let min_ref_count = 1 + external_count;
if existing_ref_count <= min_ref_count {
//RefCount 2 means that resource is hold just by DeviceTracker and this suspected resource itself
//so it's already been released from user and so it's not inside Registry\Storage
if existing_ref_count <= 2 {
self.start_set.complex.remove(&index);
self.end_set.complex.remove(&index);
self.metadata.remove(index);