Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Free the raw device when wgpu::Device is dropped. #2567

Merged
merged 1 commit into from
Apr 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions wgpu-core/src/device/life.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,11 @@ impl<A: hal::Api> LifetimeTracker<A> {
}
}

/// Return true if there are no queue submissions still in flight.
pub fn queue_empty(&self) -> bool {
self.active.is_empty()
}

/// Start tracking resources associated with a new queue submission.
pub fn track_submission(
&mut self,
Expand Down
109 changes: 86 additions & 23 deletions wgpu-core/src/device/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use crate::{
instance, pipeline, present, resource,
track::{BufferState, TextureSelector, TextureState, TrackerSet, UsageConflict},
validation::{self, check_buffer_usage, check_texture_usage},
FastHashMap, Label, LabelHelpers as _, LifeGuard, MultiRefCount, Stored, SubmissionIndex,
DOWNLEVEL_ERROR_MESSAGE,
FastHashMap, Label, LabelHelpers as _, LifeGuard, MultiRefCount, RefCount, Stored,
SubmissionIndex, DOWNLEVEL_ERROR_MESSAGE,
};

use arrayvec::ArrayVec;
Expand Down Expand Up @@ -266,6 +266,14 @@ pub struct Device<A: hal::Api> {
//desc_allocator: Mutex<descriptor::DescriptorAllocator<A>>,
//Note: The submission index here corresponds to the last submission that is done.
pub(crate) life_guard: LifeGuard,

/// A clone of `life_guard.ref_count`.
///
/// Holding a separate clone of the `RefCount` here lets us tell whether the
/// device is referenced by other resources, even if `life_guard.ref_count`
/// was set to `None` by a call to `device_drop`.
ref_count: RefCount,

command_allocator: Mutex<CommandAllocator<A>>,
pub(crate) active_submission_index: SubmissionIndex,
fence: A::Fence,
Expand Down Expand Up @@ -371,12 +379,15 @@ impl<A: HalApi> Device<A> {
}));
}

let life_guard = LifeGuard::new("<device>");
let ref_count = life_guard.add_ref();
Ok(Self {
raw: open.device,
adapter_id,
queue: open.queue,
zero_buffer,
life_guard: LifeGuard::new("<device>"),
life_guard,
ref_count,
command_allocator: Mutex::new(com_alloc),
active_submission_index: 0,
fence,
Expand Down Expand Up @@ -413,12 +424,22 @@ impl<A: HalApi> Device<A> {
self.life_tracker.lock()
}

/// Check this device for completed commands.
///
/// Return a pair `(closures, queue_empty)`, where:
///
/// - `closures` is a list of actions to take: mapping buffers, notifying the user
///
/// - `queue_empty` is a boolean indicating whether there are more queue
/// submissions still in flight. (We have to take the locks needed to
/// produce this information for other reasons, so we might as well just
/// return it to our callers.)
fn maintain<'this, 'token: 'this, G: GlobalIdentityHandlerFactory>(
&'this self,
hub: &Hub<A, G>,
force_wait: bool,
token: &mut Token<'token, Self>,
) -> Result<UserClosures, WaitIdleError> {
) -> Result<(UserClosures, bool), WaitIdleError> {
profiling::scope!("maintain", "Device");
let mut life_tracker = self.lock_life(token);

Expand Down Expand Up @@ -461,10 +482,11 @@ impl<A: HalApi> Device<A> {
let mapping_closures = life_tracker.handle_mapping(hub, &self.raw, &self.trackers, token);
life_tracker.cleanup(&self.raw);

Ok(UserClosures {
let closures = UserClosures {
mappings: mapping_closures,
submissions: submission_closures,
})
};
Ok((closures, life_tracker.queue_empty()))
}

fn untrack<'this, 'token: 'this, G: GlobalIdentityHandlerFactory>(
Expand Down Expand Up @@ -4874,7 +4896,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
device_id: id::DeviceId,
force_wait: bool,
) -> Result<(), WaitIdleError> {
let closures = {
let (closures, _) = {
let hub = A::hub(self);
let mut token = Token::root();
let (device_guard, mut token) = hub.devices.read(&mut token);
Expand All @@ -4900,12 +4922,27 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
profiling::scope!("poll_devices");

let hub = A::hub(self);
let mut token = Token::root();
let (device_guard, mut token) = hub.devices.read(&mut token);
for (_, device) in device_guard.iter(A::VARIANT) {
let cbs = device.maintain(hub, force_wait, &mut token)?;
closures.extend(cbs);
let mut devices_to_drop = vec![];
{
let mut token = Token::root();
let (device_guard, mut token) = hub.devices.read(&mut token);

for (id, device) in device_guard.iter(A::VARIANT) {
let (cbs, queue_empty) = device.maintain(hub, force_wait, &mut token)?;

// If the device's own `RefCount` clone is the only one left, and
// its submission queue is empty, then it can be freed.
if queue_empty && device.ref_count.load() == 1 {
devices_to_drop.push(id);
}
closures.extend(cbs);
}
}

for device_id in devices_to_drop {
self.exit_device::<A>(device_id);
}

Ok(())
}

Expand Down Expand Up @@ -4970,19 +5007,45 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {

let hub = A::hub(self);
let mut token = Token::root();
let (device, _) = hub.devices.unregister(device_id, &mut token);
if let Some(mut device) = device {
device.prepare_to_die();

// Adapter is only referenced by the device and itself.
// This isn't a robust way to destroy them, we should find a better one.
if device.adapter_id.ref_count.load() == 1 {
let _ = hub
.adapters
.unregister(device.adapter_id.value.0, &mut token);

// For now, just drop the `RefCount` in `device.life_guard`, which
// stands for the user's reference to the device. We'll take care of
// cleaning up the device when we're polled, once its queue submissions
// have completed and it is no longer needed by other resources.
let (mut device_guard, _) = hub.devices.write(&mut token);
if let Ok(device) = device_guard.get_mut(device_id) {
device.life_guard.ref_count.take().unwrap();
}
}

/// Exit the unreferenced, inactive device `device_id`.
fn exit_device<A: HalApi>(&self, device_id: id::DeviceId) {
let hub = A::hub(self);
let mut token = Token::root();
let mut free_adapter_id = None;
{
let (device, mut _token) = hub.devices.unregister(device_id, &mut token);
if let Some(mut device) = device {
// The things `Device::prepare_to_die` takes care are mostly
// unnecessary here. We know our queue is empty, so we don't
// need to wait for submissions or triage them. We know we were
// just polled, so `life_tracker.free_resources` is empty.
debug_assert!(device.lock_life(&mut _token).queue_empty());
device.pending_writes.deactivate();

// Adapter is only referenced by the device and itself.
// This isn't a robust way to destroy them, we should find a better one.
if device.adapter_id.ref_count.load() == 1 {
free_adapter_id = Some(device.adapter_id.value.0);
}

device.dispose();
}
}

device.dispose();
// Free the adapter now that we've dropped the `Device` token.
if let Some(free_adapter_id) = free_adapter_id {
let _ = hub.adapters.unregister(free_adapter_id, &mut token);
}
}

Expand Down
2 changes: 1 addition & 1 deletion wgpu-core/src/device/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,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, false, &mut token) {
let (closures, _) = match device.maintain(hub, false, &mut token) {
Ok(closures) => closures,
Err(WaitIdleError::Device(err)) => return Err(QueueSubmitError::Queue(err)),
Err(WaitIdleError::StuckGpu) => return Err(QueueSubmitError::StuckGpu),
Expand Down
12 changes: 4 additions & 8 deletions wgpu/src/backend/direct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1545,21 +1545,17 @@ impl crate::Context for Context {

#[cfg_attr(target_arch = "wasm32", allow(unused))]
fn device_drop(&self, device: &Self::DeviceId) {
let global = &self.0;

#[cfg(any(not(target_arch = "wasm32"), feature = "emscripten"))]
{
let global = &self.0;
match wgc::gfx_select!(device.id => global.device_poll(device.id, true)) {
Ok(()) => (),
Err(err) => self.handle_error_fatal(err, "Device::drop"),
}
}
//TODO: make this work in general
#[cfg(any(not(target_arch = "wasm32"), feature = "emscripten"))]
#[cfg(feature = "metal-auto-capture")]
{
let global = &self.0;
wgc::gfx_select!(device.id => global.device_drop(device.id));
}

wgc::gfx_select!(device.id => global.device_drop(device.id));
}

fn device_poll(&self, device: &Self::DeviceId, maintain: crate::Maintain) {
Expand Down