Skip to content

Commit

Permalink
use ManuallyDrop for Device.fence
Browse files Browse the repository at this point in the history
  • Loading branch information
teoxoy committed Aug 12, 2024
1 parent 728b288 commit 19843c9
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 27 deletions.
7 changes: 3 additions & 4 deletions wgpu-core/src/device/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1054,8 +1054,7 @@ impl Global {
let snatch_guard = device.snatchable_lock.read();

// Fence lock must be acquired after the snatch lock everywhere to avoid deadlocks.
let mut fence_guard = device.fence.write();
let fence = fence_guard.as_mut().unwrap();
let mut fence = device.fence.write();
let submit_index = device
.active_submission_index
.fetch_add(1, Ordering::SeqCst)
Expand Down Expand Up @@ -1304,7 +1303,7 @@ impl Global {
.submit(
&hal_command_buffers,
&submit_surface_textures,
(fence, submit_index),
(&mut fence, submit_index),
)
.map_err(DeviceError::from)?;
}
Expand All @@ -1327,7 +1326,7 @@ impl Global {

// 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 fence_guard = RwLockWriteGuard::downgrade(fence_guard);
let fence_guard = RwLockWriteGuard::downgrade(fence);
let (closures, _) =
match device.maintain(fence_guard, wgt::Maintain::Poll, snatch_guard) {
Ok(closures) => closures,
Expand Down
30 changes: 14 additions & 16 deletions wgpu-core/src/device/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ pub struct Device<A: HalApi> {

// NOTE: if both are needed, the `snatchable_lock` must be consistently acquired before the
// `fence` lock to avoid deadlocks.
pub(crate) fence: RwLock<Option<A::Fence>>,
pub(crate) fence: RwLock<ManuallyDrop<A::Fence>>,
pub(crate) snatchable_lock: SnatchLock,

/// Is this device valid? Valid is closely associated with "lose the device",
Expand Down Expand Up @@ -175,11 +175,13 @@ impl<A: HalApi> Drop for Device<A> {
let zero_buffer = unsafe { ManuallyDrop::take(&mut self.zero_buffer) };
// SAFETY: We are in the Drop impl and we don't use self.pending_writes anymore after this point.
let pending_writes = unsafe { ManuallyDrop::take(&mut self.pending_writes.lock()) };
// SAFETY: We are in the Drop impl and we don't use self.fence anymore after this point.
let fence = unsafe { ManuallyDrop::take(&mut self.fence.write()) };
pending_writes.dispose(&raw);
self.command_allocator.dispose(&raw);
unsafe {
raw.destroy_buffer(zero_buffer);
raw.destroy_fence(self.fence.write().take().unwrap());
raw.destroy_fence(fence);
let queue = self.queue_to_drop.take().unwrap();
raw.exit(queue);
}
Expand Down Expand Up @@ -283,7 +285,7 @@ impl<A: HalApi> Device<A> {
command_allocator,
active_submission_index: AtomicU64::new(0),
last_successful_submission_index: AtomicU64::new(0),
fence: RwLock::new(rank::DEVICE_FENCE, Some(fence)),
fence: RwLock::new(rank::DEVICE_FENCE, ManuallyDrop::new(fence)),
snatchable_lock: unsafe { SnatchLock::new(rank::DEVICE_SNATCHABLE_LOCK) },
valid: AtomicBool::new(true),
trackers: Mutex::new(rank::DEVICE_TRACKERS, DeviceTracker::new()),
Expand Down Expand Up @@ -409,14 +411,12 @@ impl<A: HalApi> Device<A> {
/// return it to our callers.)
pub(crate) fn maintain<'this>(
&'this self,
fence_guard: crate::lock::RwLockReadGuard<Option<A::Fence>>,
fence: crate::lock::RwLockReadGuard<ManuallyDrop<A::Fence>>,
maintain: wgt::Maintain<crate::SubmissionIndex>,
snatch_guard: SnatchGuard,
) -> Result<(UserClosures, bool), WaitIdleError> {
profiling::scope!("Device::maintain");

let fence = fence_guard.as_ref().unwrap();

// Determine which submission index `maintain` represents.
let submission_index = match maintain {
wgt::Maintain::WaitForSubmissionIndex(submission_index) => {
Expand All @@ -440,7 +440,7 @@ impl<A: HalApi> Device<A> {
.load(Ordering::Acquire),
wgt::Maintain::Poll => unsafe {
self.raw()
.get_fence_value(fence)
.get_fence_value(&fence)
.map_err(DeviceError::from)?
},
};
Expand All @@ -449,7 +449,7 @@ impl<A: HalApi> Device<A> {
if maintain.is_wait() {
unsafe {
self.raw()
.wait(fence, submission_index, CLEANUP_WAIT_MS)
.wait(&fence, submission_index, CLEANUP_WAIT_MS)
.map_err(DeviceError::from)?
};
}
Expand Down Expand Up @@ -490,7 +490,7 @@ impl<A: HalApi> Device<A> {

// Don't hold the locks while calling release_gpu_resources.
drop(life_tracker);
drop(fence_guard);
drop(fence);
drop(snatch_guard);

if should_release_gpu_resource {
Expand Down Expand Up @@ -3490,12 +3490,11 @@ impl<A: HalApi> Device<A> {
&self,
submission_index: crate::SubmissionIndex,
) -> Result<(), DeviceError> {
let guard = self.fence.read();
let fence = guard.as_ref().unwrap();
let last_done_index = unsafe { self.raw().get_fence_value(fence)? };
let fence = self.fence.read();
let last_done_index = unsafe { self.raw().get_fence_value(&fence)? };
if last_done_index < submission_index {
unsafe { self.raw().wait(fence, submission_index, !0)? };
drop(guard);
unsafe { self.raw().wait(&fence, submission_index, !0)? };
drop(fence);
let closures = self
.lock_life()
.triage_submissions(submission_index, &self.command_allocator);
Expand Down Expand Up @@ -3638,8 +3637,7 @@ impl<A: HalApi> Device<A> {
.load(Ordering::Acquire);
if let Err(error) = unsafe {
let fence = self.fence.read();
let fence = fence.as_ref().unwrap();
self.raw().wait(fence, current_index, CLEANUP_WAIT_MS)
self.raw().wait(&fence, current_index, CLEANUP_WAIT_MS)
} {
log::error!("failed to wait for the device: {error}");
}
Expand Down
7 changes: 3 additions & 4 deletions wgpu-core/src/present.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,18 +152,17 @@ impl Global {
});
}

let fence_guard = device.fence.read();
let fence = fence_guard.as_ref().unwrap();
let fence = device.fence.read();

let suf = A::surface_as_hal(surface.as_ref());
let (texture_id, status) = match unsafe {
suf.unwrap().acquire_texture(
Some(std::time::Duration::from_millis(FRAME_TIMEOUT_MS as u64)),
fence,
&fence,
)
} {
Ok(Some(ast)) => {
drop(fence_guard);
drop(fence);

let texture_desc = wgt::TextureDescriptor {
label: Some(std::borrow::Cow::Borrowed("<Surface Texture>")),
Expand Down
5 changes: 2 additions & 3 deletions wgpu-core/src/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1312,9 +1312,8 @@ impl Global {
let hub = A::hub(self);

if let Ok(device) = hub.devices.get(id) {
let hal_fence = device.fence.read();
let hal_fence = hal_fence.as_ref();
hal_fence_callback(hal_fence)
let fence = device.fence.read();
hal_fence_callback(Some(&fence))
} else {
hal_fence_callback(None)
}
Expand Down

0 comments on commit 19843c9

Please sign in to comment.