From e934595bb2337175a5b6ea84d33786b58820f8e3 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 15 Oct 2024 12:23:44 +0200 Subject: [PATCH] move `device_lost_closure` from `Device::life_tracker` to `Device` --- wgpu-core/src/device/global.rs | 14 +++++++------- wgpu-core/src/device/life.rs | 8 +------- wgpu-core/src/device/resource.rs | 25 ++++++++++++------------- wgpu-core/src/lock/rank.rs | 1 + 4 files changed, 21 insertions(+), 27 deletions(-) diff --git a/wgpu-core/src/device/global.rs b/wgpu-core/src/device/global.rs index c53db14be..6f0359e21 100644 --- a/wgpu-core/src/device/global.rs +++ b/wgpu-core/src/device/global.rs @@ -2090,14 +2090,14 @@ impl Global { ) { let device = self.hub.devices.get(device_id); - let mut life_tracker = device.lock_life(); - if let Some(existing_closure) = life_tracker.device_lost_closure.take() { - // It's important to not hold the lock while calling the closure. - drop(life_tracker); - existing_closure.call(DeviceLostReason::ReplacedCallback, "".to_string()); - life_tracker = device.lock_life(); + let old_device_lost_closure = device + .device_lost_closure + .lock() + .replace(device_lost_closure); + + if let Some(old_device_lost_closure) = old_device_lost_closure { + old_device_lost_closure.call(DeviceLostReason::ReplacedCallback, "".to_string()); } - life_tracker.device_lost_closure = Some(device_lost_closure); } pub fn device_destroy(&self, device_id: DeviceId) { diff --git a/wgpu-core/src/device/life.rs b/wgpu-core/src/device/life.rs index 70e5337a7..dc9d4051e 100644 --- a/wgpu-core/src/device/life.rs +++ b/wgpu-core/src/device/life.rs @@ -1,7 +1,7 @@ use crate::{ device::{ queue::{EncoderInFlight, SubmittedWorkDoneClosure, TempResource}, - DeviceError, DeviceLostClosure, + DeviceError, }, resource::{self, Buffer, Texture, Trackable}, snatch::SnatchGuard, @@ -196,11 +196,6 @@ pub(crate) struct LifetimeTracker { /// must happen _after_ all mapped buffer callbacks are mapped, so we defer them /// here until the next time the device is maintained. work_done_closures: SmallVec<[SubmittedWorkDoneClosure; 1]>, - - /// Closure to be called on "lose the device". This is invoked directly by - /// device.lose or by the UserCallbacks returned from maintain when the device - /// has been destroyed and its queues are empty. - pub device_lost_closure: Option, } impl LifetimeTracker { @@ -209,7 +204,6 @@ impl LifetimeTracker { active: Vec::new(), ready_to_map: Vec::new(), work_done_closures: SmallVec::new(), - device_lost_closure: None, } } diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index eba5a30bb..5f50feed8 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -51,8 +51,8 @@ use std::{ }; use super::{ - queue::Queue, DeviceDescriptor, DeviceError, UserClosures, ENTRYPOINT_FAILURE_ERROR, - ZERO_BUFFER_SIZE, + queue::Queue, DeviceDescriptor, DeviceError, DeviceLostClosure, UserClosures, + ENTRYPOINT_FAILURE_ERROR, ZERO_BUFFER_SIZE, }; /// Structure describing a logical device. Some members are internally mutable, @@ -106,6 +106,11 @@ pub struct Device { /// using ref-counted references for internal access. pub(crate) valid: AtomicBool, + /// Closure to be called on "lose the device". This is invoked directly by + /// device.lose or by the UserCallbacks returned from maintain when the device + /// has been destroyed and its queues are empty. + pub(crate) device_lost_closure: Mutex>, + /// Stores the state of buffers and textures. pub(crate) trackers: Mutex, pub(crate) tracker_indices: TrackerIndexAllocators, @@ -147,8 +152,7 @@ impl Drop for Device { fn drop(&mut self) { resource_log!("Drop {}", self.error_ident()); - let device_lost_closure = self.lock_life().device_lost_closure.take(); - if let Some(closure) = device_lost_closure { + if let Some(closure) = self.device_lost_closure.lock().take() { closure.call(DeviceLostReason::Dropped, String::from("Device dropped.")); } @@ -259,6 +263,7 @@ impl Device { fence: RwLock::new(rank::DEVICE_FENCE, ManuallyDrop::new(fence)), snatchable_lock: unsafe { SnatchLock::new(rank::DEVICE_SNATCHABLE_LOCK) }, valid: AtomicBool::new(true), + device_lost_closure: Mutex::new(rank::DEVICE_LOST_CLOSURE, None), trackers: Mutex::new(rank::DEVICE_TRACKERS, DeviceTracker::new()), tracker_indices: TrackerIndexAllocators::new(), life_tracker: Mutex::new(rank::DEVICE_LIFE_TRACKER, LifetimeTracker::new()), @@ -466,9 +471,9 @@ impl Device { // If we have a DeviceLostClosure, build an invocation with the // reason DeviceLostReason::Destroyed and no message. - if life_tracker.device_lost_closure.is_some() { + if let Some(device_lost_closure) = self.device_lost_closure.lock().take() { device_lost_invocations.push(DeviceLostInvocation { - closure: life_tracker.device_lost_closure.take().unwrap(), + closure: device_lost_closure, reason: DeviceLostReason::Destroyed, message: String::new(), }); @@ -3623,13 +3628,7 @@ impl Device { self.valid.store(false, Ordering::Release); // 1) Resolve the GPUDevice device.lost promise. - let mut life_lock = self.lock_life(); - let closure = life_lock.device_lost_closure.take(); - // It's important to not hold the lock while calling the closure and while calling - // release_gpu_resources which may take the lock again. - drop(life_lock); - - if let Some(device_lost_closure) = closure { + if let Some(device_lost_closure) = self.device_lost_closure.lock().take() { device_lost_closure.call(DeviceLostReason::Unknown, message.to_string()); } diff --git a/wgpu-core/src/lock/rank.rs b/wgpu-core/src/lock/rank.rs index 154d82b8d..1e8b4d2ae 100644 --- a/wgpu-core/src/lock/rank.rs +++ b/wgpu-core/src/lock/rank.rs @@ -135,6 +135,7 @@ define_lock_ranks! { #[allow(dead_code)] rank DEVICE_TRACE "Device::trace" followed by { } rank DEVICE_TRACKERS "Device::trackers" followed by { } + rank DEVICE_LOST_CLOSURE "Device::device_lost_closure" followed by { } rank DEVICE_USAGE_SCOPES "Device::usage_scopes" followed by { } rank IDENTITY_MANAGER_VALUES "IdentityManager::values" followed by { } rank REGISTRY_STORAGE "Registry::storage" followed by { }