Skip to content

Commit

Permalink
Move Snatchable to its own module
Browse files Browse the repository at this point in the history
  • Loading branch information
nical committed Dec 13, 2023
1 parent d9ce309 commit 04a5d1d
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 64 deletions.
2 changes: 1 addition & 1 deletion wgpu-core/src/command/compute.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::device::resource::SnatchGuard;
use crate::resource::Resource;
use crate::snatch::SnatchGuard;
use crate::{
binding_model::{
BindError, BindGroup, LateMinBufferBindingSizeMismatch, PushConstantUploadError,
Expand Down
2 changes: 1 addition & 1 deletion wgpu-core/src/command/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ pub use self::{

use self::memory_init::CommandBufferTextureMemoryActions;

use crate::device::resource::SnatchGuard;
use crate::device::Device;
use crate::error::{ErrorFormatter, PrettyError};
use crate::hub::Hub;
use crate::id::CommandBufferId;
use crate::snatch::SnatchGuard;

use crate::init_tracker::BufferInitTrackerAction;
use crate::resource::{Resource, ResourceInfo, ResourceType};
Expand Down
62 changes: 5 additions & 57 deletions wgpu-core/src/device/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use crate::{
TextureViewNotRenderableReason,
},
resource_log,
snatch::{SnatchGuard, SnatchLock, Snatchable},
storage::Storage,
track::{BindGroupStates, TextureSelector, Tracker},
validation::{self, check_buffer_usage, check_texture_usage},
Expand All @@ -34,15 +35,14 @@ use crate::{

use arrayvec::ArrayVec;
use hal::{CommandEncoder as _, Device as _};
use parking_lot::{Mutex, MutexGuard, RwLock, RwLockReadGuard, RwLockWriteGuard};
use parking_lot::{Mutex, MutexGuard, RwLock};

use smallvec::SmallVec;
use thiserror::Error;
use wgt::{DeviceLostReason, TextureFormat, TextureSampleType, TextureViewDimension};

use std::{
borrow::Cow,
cell::UnsafeCell,
iter,
num::NonZeroU32,
sync::{
Expand Down Expand Up @@ -95,7 +95,7 @@ pub struct Device<A: HalApi> {
//Note: The submission index here corresponds to the last submission that is done.
pub(crate) active_submission_index: AtomicU64, //SubmissionIndex,
pub(crate) fence: RwLock<Option<A::Fence>>,
pub(crate) snatchable_lock: RwLock<SnatchableMarker>,
pub(crate) snatchable_lock: SnatchLock,

/// Is this device valid? Valid is closely associated with "lose the device",
/// which can be triggered by various methods, including at the end of device
Expand Down Expand Up @@ -256,7 +256,7 @@ impl<A: HalApi> Device<A> {
command_allocator: Mutex::new(Some(com_alloc)),
active_submission_index: AtomicU64::new(0),
fence: RwLock::new(Some(fence)),
snatchable_lock: RwLock::new(SnatchableMarker),
snatchable_lock: unsafe { SnatchLock::new() },
valid: AtomicBool::new(true),
trackers: Mutex::new(Tracker::new()),
life_tracker: Mutex::new(life::LifetimeTracker::new()),
Expand Down Expand Up @@ -1769,7 +1769,7 @@ impl<A: HalApi> Device<A> {
used: &mut BindGroupStates<A>,
storage: &'a Storage<Buffer<A>, id::BufferId>,
limits: &wgt::Limits,
snatch_guard: &'a RwLockReadGuard<'a, SnatchableMarker>,
snatch_guard: &'a SnatchGuard<'a>,
) -> Result<hal::BufferBinding<'a, A>, binding_model::CreateBindGroupError> {
use crate::binding_model::CreateBindGroupError as Error;

Expand Down Expand Up @@ -3413,55 +3413,3 @@ impl<A: HalApi> Resource<DeviceId> for Device<A> {
&mut self.info
}
}

pub struct SnatchableMarker;
/// A guard that provides read access to snatchable data.
pub type SnatchGuard<'a> = RwLockReadGuard<'a, SnatchableMarker>;
/// A guard that allows snatching the snatchable data.
pub type ExclusiveSnatchGuard<'a> = RwLockWriteGuard<'a, SnatchableMarker>;

/// A value that is mostly immutable but can be "snatched" if we need to destroy
/// it early.
///
/// In order to safely access the underlying data, the device's global snatchable
/// lock must be taken. To guarentee it, methods take a read or write guard of that
/// special lock.
pub struct Snatchable<T> {
value: UnsafeCell<Option<T>>,
}

impl<T> Snatchable<T> {
pub fn new(val: T) -> Self {
Snatchable {
value: UnsafeCell::new(Some(val)),
}
}

/// Get read access to the value. Requires a the snatchable lock's read guard.
pub fn get(&self, _guard: &SnatchGuard) -> Option<&T> {
unsafe { (*self.value.get()).as_ref() }
}

/// Take the value. Requires a the snatchable lock's write guard.
pub fn snatch(&self, _guard: ExclusiveSnatchGuard) -> Option<T> {
unsafe { (*self.value.get()).take() }
}

/// Take the value without a guard. This can only be used with exclusive access
/// to self, so it does not require locking.
///
/// Typically useful in a drop implementation.
pub fn take(&mut self) -> Option<T> {
self.value.get_mut().take()
}
}

// Can't safely print the contents of a snatchable objetc without holding
// the lock.
impl<T> std::fmt::Debug for Snatchable<T> {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(f, "<snatchable>")
}
}

unsafe impl<T> Sync for Snatchable<T> {}
1 change: 1 addition & 0 deletions wgpu-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ pub mod pipeline;
pub mod present;
pub mod registry;
pub mod resource;
mod snatch;
pub mod storage;
mod track;
// This is public for users who pre-compile shaders while still wanting to
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 @@ -2,9 +2,7 @@
use crate::device::trace;
use crate::{
device::{
queue,
resource::{SnatchGuard, Snatchable},
BufferMapPendingClosure, Device, DeviceError, HostMap, MissingDownlevelFlags,
queue, BufferMapPendingClosure, Device, DeviceError, HostMap, MissingDownlevelFlags,
MissingFeatures,
},
global::Global,
Expand All @@ -16,6 +14,7 @@ use crate::{
identity::{GlobalIdentityHandlerFactory, IdentityManager},
init_tracker::{BufferInitTracker, TextureInitTracker},
resource, resource_log,
snatch::{SnatchGuard, Snatchable},
track::TextureSelector,
validation::MissingBufferUsageError,
Label, SubmissionIndex,
Expand Down
87 changes: 87 additions & 0 deletions wgpu-core/src/snatch.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
#![allow(unused)]

use std::cell::UnsafeCell;
use parking_lot::{RwLockReadGuard, RwLockWriteGuard, RwLock};

/// A guard that provides read access to snatchable data.
pub struct SnatchGuard<'a>(RwLockReadGuard<'a, ()>);
/// A guard that allows snatching the snatchable data.
pub struct ExclusiveSnatchGuard<'a>(RwLockWriteGuard<'a, ()>);

/// A value that is mostly immutable but can be "snatched" if we need to destroy
/// it early.
///
/// In order to safely access the underlying data, the device's global snatchable
/// lock must be taken. To guarentee it, methods take a read or write guard of that
/// special lock.
pub struct Snatchable<T> {
value: UnsafeCell<Option<T>>,
}

impl<T> Snatchable<T> {
pub fn new(val: T) -> Self {
Snatchable {
value: UnsafeCell::new(Some(val)),
}
}

/// Get read access to the value. Requires a the snatchable lock's read guard.
pub fn get(&self, _guard: &SnatchGuard) -> Option<&T> {
unsafe { (*self.value.get()).as_ref() }
}

/// Take the value. Requires a the snatchable lock's write guard.
pub fn snatch(&self, _guard: ExclusiveSnatchGuard) -> Option<T> {
unsafe { (*self.value.get()).take() }
}

/// Take the value without a guard. This can only be used with exclusive access
/// to self, so it does not require locking.
///
/// Typically useful in a drop implementation.
pub fn take(&mut self) -> Option<T> {
self.value.get_mut().take()
}
}

// Can't safely print the contents of a snatchable objetc without holding
// the lock.
impl<T> std::fmt::Debug for Snatchable<T> {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(f, "<snatchable>")
}
}

unsafe impl<T> Sync for Snatchable<T> {}


/// A Device-global lock for all snatchable data.
pub struct SnatchLock {
lock: RwLock<()>,
}

impl SnatchLock {
/// The safety of `Snatchable::get` and `Snatchable::snatch` rely on their using the
/// right SnatchLock (the one associated to the same device). This method is unsafe
/// to force force sers to think twice about creating a SnatchLock. The only place this
/// method sould be called is when creating the device.
pub unsafe fn new() -> Self {
SnatchLock {
lock: RwLock::new(()),
}
}

/// Request read access to snatchable resources.
pub fn read(&self) -> SnatchGuard {
SnatchGuard(self.lock.read())
}

/// Request write access to snatchable resources.
///
/// This should only be called when a resource needs to be snatched. This has
/// a high risk of causing lock contention if called concurrently with other
/// wgpu work.
pub fn write(&self) -> ExclusiveSnatchGuard {
ExclusiveSnatchGuard(self.lock.write())
}
}
2 changes: 1 addition & 1 deletion wgpu-core/src/track/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ use std::{borrow::Cow, marker::PhantomData, sync::Arc};

use super::{PendingTransition, ResourceTracker};
use crate::{
device::resource::SnatchGuard,
hal_api::HalApi,
id::{BufferId, TypedId},
resource::{Buffer, Resource},
snatch::SnatchGuard,
storage::Storage,
track::{
invalid_resource_state, skip_barrier, ResourceMetadata, ResourceMetadataProvider,
Expand Down
2 changes: 1 addition & 1 deletion wgpu-core/src/track/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ mod texture;

use crate::{
binding_model, command, conv,
device::resource::SnatchGuard,
hal_api::HalApi,
id::{self, TypedId},
pipeline, resource,
snatch::SnatchGuard,
storage::Storage,
};

Expand Down

0 comments on commit 04a5d1d

Please sign in to comment.