Skip to content

Commit

Permalink
Store the device's queue via a weak ref instead of an ID (#5230)
Browse files Browse the repository at this point in the history
  • Loading branch information
nical authored Feb 15, 2024
1 parent abc0b30 commit 286306d
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 17 deletions.
5 changes: 2 additions & 3 deletions wgpu-core/src/device/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1330,9 +1330,8 @@ impl Global {
if !device.is_valid() {
break DeviceError::Lost;
}
let queue = match hub.queues.get(device.queue_id.read().unwrap()) {
Ok(queue) => queue,
Err(_) => break DeviceError::InvalidQueueId,
let Some(queue) = device.get_queue() else {
break DeviceError::InvalidQueueId;
};
let encoder = match device
.command_allocator
Expand Down
24 changes: 16 additions & 8 deletions wgpu-core/src/device/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use crate::{
hal_api::HalApi,
hal_label,
hub::Hub,
id::QueueId,
init_tracker::{
BufferInitTracker, BufferInitTrackerAction, MemoryInitKind, TextureInitRange,
TextureInitTracker, TextureInitTrackerAction,
Expand All @@ -38,6 +37,7 @@ use crate::{

use arrayvec::ArrayVec;
use hal::{CommandEncoder as _, Device as _};
use once_cell::sync::OnceCell;
use parking_lot::{Mutex, MutexGuard, RwLock};

use smallvec::SmallVec;
Expand All @@ -56,7 +56,7 @@ use std::{

use super::{
life::{self, ResourceMaps},
queue::{self},
queue::{self, Queue},
DeviceDescriptor, DeviceError, ImplicitPipelineContext, UserClosures, ENTRYPOINT_FAILURE_ERROR,
IMPLICIT_BIND_GROUP_LAYOUT_ERROR_LABEL, ZERO_BUFFER_SIZE,
};
Expand Down Expand Up @@ -89,8 +89,8 @@ use super::{
pub struct Device<A: HalApi> {
raw: Option<A::Device>,
pub(crate) adapter: Arc<Adapter<A>>,
pub(crate) queue_id: RwLock<Option<QueueId>>,
queue_to_drop: RwLock<Option<A::Queue>>,
pub(crate) queue: OnceCell<Weak<Queue<A>>>,
queue_to_drop: OnceCell<A::Queue>,
pub(crate) zero_buffer: Option<A::Buffer>,
pub(crate) info: ResourceInfo<Device<A>>,

Expand Down Expand Up @@ -162,7 +162,7 @@ impl<A: HalApi> Drop for Device<A> {
unsafe {
raw.destroy_buffer(self.zero_buffer.take().unwrap());
raw.destroy_fence(self.fence.write().take().unwrap());
let queue = self.queue_to_drop.write().take().unwrap();
let queue = self.queue_to_drop.take().unwrap();
raw.exit(queue);
}
}
Expand Down Expand Up @@ -260,8 +260,8 @@ impl<A: HalApi> Device<A> {
Ok(Self {
raw: Some(raw_device),
adapter: adapter.clone(),
queue_id: RwLock::new(None),
queue_to_drop: RwLock::new(None),
queue: OnceCell::new(),
queue_to_drop: OnceCell::new(),
zero_buffer: Some(zero_buffer),
info: ResourceInfo::new("<device>"),
command_allocator: Mutex::new(Some(com_alloc)),
Expand Down Expand Up @@ -302,7 +302,7 @@ impl<A: HalApi> Device<A> {
}

pub(crate) fn release_queue(&self, queue: A::Queue) {
self.queue_to_drop.write().replace(queue);
assert!(self.queue_to_drop.set(queue).is_ok());
}

pub(crate) fn lock_life<'a>(&'a self) -> MutexGuard<'a, LifetimeTracker<A>> {
Expand Down Expand Up @@ -359,6 +359,14 @@ impl<A: HalApi> Device<A> {
}
}

pub fn get_queue(&self) -> Option<Arc<Queue<A>>> {
self.queue.get().as_ref()?.upgrade()
}

pub fn set_queue(&self, queue: Arc<Queue<A>>) {
assert!(self.queue.set(Arc::downgrade(&queue)).is_ok());
}

/// Check this device for completed commands.
///
/// The `maintain` argument tells how the maintence function should behave, either
Expand Down
8 changes: 4 additions & 4 deletions wgpu-core/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1072,10 +1072,10 @@ impl Global {
let device = hub.devices.get(device_id).unwrap();
queue.device = Some(device.clone());

let (queue_id, _) = queue_fid.assign(queue);
let (queue_id, queue) = queue_fid.assign(queue);
resource_log!("Created Queue {:?}", queue_id);

device.queue_id.write().replace(queue_id);
device.set_queue(queue);

return (device_id, queue_id, None);
};
Expand Down Expand Up @@ -1124,10 +1124,10 @@ impl Global {
let device = hub.devices.get(device_id).unwrap();
queue.device = Some(device.clone());

let (queue_id, _) = queues_fid.assign(queue);
let (queue_id, queue) = queues_fid.assign(queue);
resource_log!("Created Queue {:?}", queue_id);

device.queue_id.write().replace(queue_id);
device.set_queue(queue);

return (device_id, queue_id, None);
};
Expand Down
3 changes: 1 addition & 2 deletions wgpu-core/src/present.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,7 @@ impl Global {
if !device.is_valid() {
return Err(DeviceError::Lost.into());
}
let queue_id = device.queue_id.read().unwrap();
let queue = hub.queues.get(queue_id).unwrap();
let queue = device.get_queue().unwrap();

#[cfg(feature = "trace")]
if let Some(ref mut trace) = *device.trace.lock() {
Expand Down

0 comments on commit 286306d

Please sign in to comment.