[wgpu-core] use .strict_get() & .strict_unregister() for devices

This works because we never assign errors to devices (they are never invalid).
This commit is contained in:
teoxoy 2024-09-05 15:28:20 +02:00 committed by Teodor Tanasoaia
parent 07c397a1e5
commit 3437589107
11 changed files with 100 additions and 303 deletions

View File

@ -663,9 +663,9 @@ pub fn op_webgpu_request_device(
let (device, queue) = res.map_err(|err| DomExceptionOperationError::new(&err.to_string()))?;
let device_features = instance.device_features(device)?;
let device_features = instance.device_features(device);
let features = deserialize_features(&device_features);
let limits = instance.device_limits(device)?;
let limits = instance.device_limits(device);
let instance = instance.clone();
let instance2 = instance.clone();

View File

@ -651,33 +651,6 @@ static DEVICE_DROP_THEN_LOST: GpuTestConfiguration = GpuTestConfiguration::new()
);
});
#[gpu_test]
static DEVICE_INVALID_THEN_SET_LOST_CALLBACK: GpuTestConfiguration = GpuTestConfiguration::new()
.parameters(TestParameters::default().expect_fail(FailureCase::webgl2()))
.run_sync(|ctx| {
// This test checks that when the device is invalid, a subsequent call
// to set the device lost callback will immediately call the callback.
// Invalidating the device is done via a testing-only method. Fails on
// webgl because webgl doesn't implement make_invalid.
// Make the device invalid.
ctx.device.make_invalid();
static WAS_CALLED: AtomicBool = AtomicBool::new(false);
// Set a LoseDeviceCallback on the device.
let callback = Box::new(|reason, _m| {
WAS_CALLED.store(true, std::sync::atomic::Ordering::SeqCst);
assert_eq!(reason, wgt::DeviceLostReason::DeviceInvalid);
});
ctx.device.set_device_lost_callback(callback);
assert!(
WAS_CALLED.load(std::sync::atomic::Ordering::SeqCst),
"Device lost callback should have been called."
);
});
#[gpu_test]
static DEVICE_LOST_REPLACED_CALLBACK: GpuTestConfiguration = GpuTestConfiguration::new()
.parameters(TestParameters::default())

View File

@ -78,40 +78,19 @@ impl Global {
get_supported_callback(&adapter, &surface)
}
pub fn device_features(&self, device_id: DeviceId) -> Result<wgt::Features, DeviceError> {
let hub = &self.hub;
let device = hub
.devices
.get(device_id)
.map_err(|_| DeviceError::InvalidDeviceId)?;
Ok(device.features)
pub fn device_features(&self, device_id: DeviceId) -> wgt::Features {
let device = self.hub.devices.strict_get(device_id);
device.features
}
pub fn device_limits(&self, device_id: DeviceId) -> Result<wgt::Limits, DeviceError> {
let hub = &self.hub;
let device = hub
.devices
.get(device_id)
.map_err(|_| DeviceError::InvalidDeviceId)?;
Ok(device.limits.clone())
pub fn device_limits(&self, device_id: DeviceId) -> wgt::Limits {
let device = self.hub.devices.strict_get(device_id);
device.limits.clone()
}
pub fn device_downlevel_properties(
&self,
device_id: DeviceId,
) -> Result<wgt::DownlevelCapabilities, DeviceError> {
let hub = &self.hub;
let device = hub
.devices
.get(device_id)
.map_err(|_| DeviceError::InvalidDeviceId)?;
Ok(device.downlevel.clone())
pub fn device_downlevel_properties(&self, device_id: DeviceId) -> wgt::DownlevelCapabilities {
let device = self.hub.devices.strict_get(device_id);
device.downlevel.clone()
}
pub fn device_create_buffer(
@ -126,12 +105,7 @@ impl Global {
let fid = hub.buffers.prepare(device_id.backend(), id_in);
let error = 'error: {
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => {
break 'error DeviceError::InvalidDeviceId.into();
}
};
let device = self.hub.devices.strict_get(device_id);
#[cfg(feature = "trace")]
if let Some(ref mut trace) = *device.trace.lock() {
@ -339,10 +313,7 @@ impl Global {
let fid = hub.textures.prepare(device_id.backend(), id_in);
let error = 'error: {
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => break 'error DeviceError::InvalidDeviceId.into(),
};
let device = self.hub.devices.strict_get(device_id);
#[cfg(feature = "trace")]
if let Some(ref mut trace) = *device.trace.lock() {
@ -385,10 +356,7 @@ impl Global {
let fid = hub.textures.prepare(device_id.backend(), id_in);
let error = 'error: {
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => break 'error DeviceError::InvalidDeviceId.into(),
};
let device = self.hub.devices.strict_get(device_id);
// NB: Any change done through the raw texture handle will not be
// recorded in the replay
@ -431,31 +399,21 @@ impl Global {
let hub = &self.hub;
let fid = hub.buffers.prepare(A::VARIANT, id_in);
let error = 'error: {
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => break 'error DeviceError::InvalidDeviceId.into(),
};
let device = self.hub.devices.strict_get(device_id);
// NB: Any change done through the raw buffer handle will not be
// recorded in the replay
#[cfg(feature = "trace")]
if let Some(trace) = device.trace.lock().as_mut() {
trace.add(trace::Action::CreateBuffer(fid.id(), desc.clone()));
}
// NB: Any change done through the raw buffer handle will not be
// recorded in the replay
#[cfg(feature = "trace")]
if let Some(trace) = device.trace.lock().as_mut() {
trace.add(trace::Action::CreateBuffer(fid.id(), desc.clone()));
}
let buffer = device.create_buffer_from_hal(Box::new(hal_buffer), desc);
let buffer = device.create_buffer_from_hal(Box::new(hal_buffer), desc);
let id = fid.assign(buffer);
api_log!("Device::create_buffer -> {id:?}");
let id = fid.assign(buffer);
api_log!("Device::create_buffer -> {id:?}");
return (id, None);
};
log::error!("Device::create_buffer error: {error}");
let id = fid.assign_error();
(id, Some(error))
(id, None)
}
pub fn texture_destroy(&self, texture_id: id::TextureId) -> Result<(), resource::DestroyError> {
@ -568,10 +526,7 @@ impl Global {
let fid = hub.samplers.prepare(device_id.backend(), id_in);
let error = 'error: {
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => break 'error DeviceError::InvalidDeviceId.into(),
};
let device = self.hub.devices.strict_get(device_id);
#[cfg(feature = "trace")]
if let Some(ref mut trace) = *device.trace.lock() {
@ -622,10 +577,7 @@ impl Global {
let fid = hub.bind_group_layouts.prepare(device_id.backend(), id_in);
let error = 'error: {
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => break 'error DeviceError::InvalidDeviceId.into(),
};
let device = self.hub.devices.strict_get(device_id);
#[cfg(feature = "trace")]
if let Some(ref mut trace) = *device.trace.lock() {
@ -696,10 +648,7 @@ impl Global {
let fid = hub.pipeline_layouts.prepare(device_id.backend(), id_in);
let error = 'error: {
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => break 'error DeviceError::InvalidDeviceId.into(),
};
let device = self.hub.devices.strict_get(device_id);
#[cfg(feature = "trace")]
if let Some(ref mut trace) = *device.trace.lock() {
@ -770,10 +719,7 @@ impl Global {
let fid = hub.bind_groups.prepare(device_id.backend(), id_in);
let error = 'error: {
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => break 'error DeviceError::InvalidDeviceId.into(),
};
let device = self.hub.devices.strict_get(device_id);
#[cfg(feature = "trace")]
if let Some(ref mut trace) = *device.trace.lock() {
@ -932,10 +878,7 @@ impl Global {
let fid = hub.shader_modules.prepare(device_id.backend(), id_in);
let error = 'error: {
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => break 'error DeviceError::InvalidDeviceId.into(),
};
let device = self.hub.devices.strict_get(device_id);
#[cfg(feature = "trace")]
if let Some(ref mut trace) = *device.trace.lock() {
@ -1007,10 +950,7 @@ impl Global {
let fid = hub.shader_modules.prepare(device_id.backend(), id_in);
let error = 'error: {
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => break 'error DeviceError::InvalidDeviceId.into(),
};
let device = self.hub.devices.strict_get(device_id);
#[cfg(feature = "trace")]
if let Some(ref mut trace) = *device.trace.lock() {
@ -1069,10 +1009,7 @@ impl Global {
);
let error = 'error: {
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => break 'error DeviceError::InvalidDeviceId,
};
let device = self.hub.devices.strict_get(device_id);
let command_buffer = match device.create_command_encoder(&desc.label) {
Ok(command_buffer) => command_buffer,
@ -1140,14 +1077,7 @@ impl Global {
.prepare(bundle_encoder.parent().backend(), id_in);
let error = 'error: {
let device = match hub.devices.get(bundle_encoder.parent()) {
Ok(device) => device,
Err(_) => {
break 'error command::RenderBundleError::from_device_error(
DeviceError::InvalidDeviceId,
);
}
};
let device = self.hub.devices.strict_get(bundle_encoder.parent());
#[cfg(feature = "trace")]
if let Some(ref mut trace) = *device.trace.lock() {
@ -1204,10 +1134,7 @@ impl Global {
let fid = hub.query_sets.prepare(device_id.backend(), id_in);
let error = 'error: {
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => break 'error DeviceError::InvalidDeviceId.into(),
};
let device = self.hub.devices.strict_get(device_id);
#[cfg(feature = "trace")]
if let Some(ref mut trace) = *device.trace.lock() {
@ -1272,10 +1199,7 @@ impl Global {
break 'error pipeline::ImplicitLayoutError::MissingImplicitPipelineIds.into();
}
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => break 'error DeviceError::InvalidDeviceId.into(),
};
let device = self.hub.devices.strict_get(device_id);
#[cfg(feature = "trace")]
if let Some(ref mut trace) = *device.trace.lock() {
@ -1519,10 +1443,7 @@ impl Global {
break 'error pipeline::ImplicitLayoutError::MissingImplicitPipelineIds.into();
}
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => break 'error DeviceError::InvalidDeviceId.into(),
};
let device = self.hub.devices.strict_get(device_id);
#[cfg(feature = "trace")]
if let Some(ref mut trace) = *device.trace.lock() {
@ -1713,11 +1634,7 @@ impl Global {
let fid = hub.pipeline_caches.prepare(device_id.backend(), id_in);
let error: pipeline::CreatePipelineCacheError = 'error: {
let device = match hub.devices.get(device_id) {
Ok(device) => device,
// TODO: Handle error properly
Err(crate::storage::InvalidId) => break 'error DeviceError::InvalidDeviceId.into(),
};
let device = self.hub.devices.strict_get(device_id);
#[cfg(feature = "trace")]
if let Some(ref mut trace) = *device.trace.lock() {
@ -1885,12 +1802,7 @@ impl Global {
// User callbacks must not be called while we are holding locks.
let user_callbacks;
{
let hub = &self.hub;
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => break 'error DeviceError::InvalidDeviceId.into(),
};
let device = self.hub.devices.strict_get(device_id);
#[cfg(feature = "trace")]
if let Some(ref mut trace) = *device.trace.lock() {
@ -2028,11 +1940,7 @@ impl Global {
) -> Result<bool, WaitIdleError> {
api_log!("Device::poll {maintain:?}");
let hub = &self.hub;
let device = hub
.devices
.get(device_id)
.map_err(|_| DeviceError::InvalidDeviceId)?;
let device = self.hub.devices.strict_get(device_id);
let DevicePoll {
closures,
@ -2139,38 +2047,26 @@ impl Global {
Ok(all_queue_empty)
}
pub fn device_start_capture(&self, id: DeviceId) {
pub fn device_start_capture(&self, device_id: DeviceId) {
api_log!("Device::start_capture");
let hub = &self.hub;
let device = self.hub.devices.strict_get(device_id);
if let Ok(device) = hub.devices.get(id) {
if !device.is_valid() {
return;
}
unsafe { device.raw().start_capture() };
if !device.is_valid() {
return;
}
unsafe { device.raw().start_capture() };
}
pub fn device_stop_capture(&self, id: DeviceId) {
pub fn device_stop_capture(&self, device_id: DeviceId) {
api_log!("Device::stop_capture");
let hub = &self.hub;
let device = self.hub.devices.strict_get(device_id);
if let Ok(device) = hub.devices.get(id) {
if !device.is_valid() {
return;
}
unsafe { device.raw().stop_capture() };
if !device.is_valid() {
return;
}
}
// This is a test-only function to force the device into an
// invalid state by inserting an error value in its place in
// the registry.
pub fn device_make_invalid(&self, device_id: DeviceId) {
let hub = &self.hub;
hub.devices.force_replace_with_error(device_id);
unsafe { device.raw().stop_capture() };
}
pub fn pipeline_cache_get_data(&self, id: id::PipelineCacheId) -> Option<Vec<u8>> {
@ -2206,22 +2102,20 @@ impl Global {
profiling::scope!("Device::drop");
api_log!("Device::drop {device_id:?}");
let hub = &self.hub;
if let Some(device) = hub.devices.unregister(device_id) {
let device_lost_closure = device.lock_life().device_lost_closure.take();
if let Some(closure) = device_lost_closure {
closure.call(DeviceLostReason::Dropped, String::from("Device dropped."));
}
// 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().queue_empty());
device.pending_writes.lock().deactivate();
drop(device);
let device = self.hub.devices.strict_unregister(device_id);
let device_lost_closure = device.lock_life().device_lost_closure.take();
if let Some(closure) = device_lost_closure {
closure.call(DeviceLostReason::Dropped, String::from("Device dropped."));
}
// 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().queue_empty());
device.pending_writes.lock().deactivate();
drop(device);
}
// This closure will be called exactly once during "lose the device",
@ -2231,61 +2125,47 @@ impl Global {
device_id: DeviceId,
device_lost_closure: DeviceLostClosure,
) {
let hub = &self.hub;
let device = self.hub.devices.strict_get(device_id);
if let Ok(device) = 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();
}
life_tracker.device_lost_closure = Some(device_lost_closure);
} else {
// No device? Okay. Just like we have to call any existing closure
// before we drop it, we need to call this closure before we exit
// this function, because there's no device that is ever going to
// call it.
device_lost_closure.call(DeviceLostReason::DeviceInvalid, "".to_string());
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();
}
life_tracker.device_lost_closure = Some(device_lost_closure);
}
pub fn device_destroy(&self, device_id: DeviceId) {
api_log!("Device::destroy {device_id:?}");
let hub = &self.hub;
let device = self.hub.devices.strict_get(device_id);
if let Ok(device) = hub.devices.get(device_id) {
// Follow the steps at
// https://gpuweb.github.io/gpuweb/#dom-gpudevice-destroy.
// It's legal to call destroy multiple times, but if the device
// is already invalid, there's nothing more to do. There's also
// no need to return an error.
if !device.is_valid() {
return;
}
// The last part of destroy is to lose the device. The spec says
// delay that until all "currently-enqueued operations on any
// queue on this device are completed." This is accomplished by
// setting valid to false, and then relying upon maintain to
// check for empty queues and a DeviceLostClosure. At that time,
// the DeviceLostClosure will be called with "destroyed" as the
// reason.
device.valid.store(false, Ordering::Release);
// Follow the steps at
// https://gpuweb.github.io/gpuweb/#dom-gpudevice-destroy.
// It's legal to call destroy multiple times, but if the device
// is already invalid, there's nothing more to do. There's also
// no need to return an error.
if !device.is_valid() {
return;
}
// The last part of destroy is to lose the device. The spec says
// delay that until all "currently-enqueued operations on any
// queue on this device are completed." This is accomplished by
// setting valid to false, and then relying upon maintain to
// check for empty queues and a DeviceLostClosure. At that time,
// the DeviceLostClosure will be called with "destroyed" as the
// reason.
device.valid.store(false, Ordering::Release);
}
pub fn device_get_internal_counters(&self, device_id: DeviceId) -> wgt::InternalCounters {
let hub = &self.hub;
if let Ok(device) = hub.devices.get(device_id) {
wgt::InternalCounters {
hal: device.get_hal_counters(),
core: wgt::CoreCounters {},
}
} else {
Default::default()
let device = self.hub.devices.strict_get(device_id);
wgt::InternalCounters {
hal: device.get_hal_counters(),
core: wgt::CoreCounters {},
}
}
@ -2293,11 +2173,8 @@ impl Global {
&self,
device_id: DeviceId,
) -> Option<wgt::AllocatorReport> {
let hub = &self.hub;
hub.devices
.get(device_id)
.ok()
.and_then(|device| device.generate_allocator_report())
let device = self.hub.devices.strict_get(device_id);
device.generate_allocator_report()
}
pub fn queue_drop(&self, queue_id: QueueId) {

View File

@ -414,8 +414,6 @@ pub enum DeviceError {
OutOfMemory,
#[error("Creation of a resource failed for a reason other than running out of memory.")]
ResourceCreationFailed,
#[error("DeviceId is invalid")]
InvalidDeviceId,
#[error(transparent)]
DeviceMismatch(#[from] Box<DeviceMismatch>),
}

View File

@ -106,11 +106,6 @@ impl<T: StorageItem> Registry<T> {
pub(crate) fn write<'a>(&'a self) -> RwLockWriteGuard<'a, Storage<T>> {
self.storage.write()
}
pub(crate) fn force_replace_with_error(&self, id: Id<T::Marker>) {
let mut storage = self.storage.write();
storage.remove(id);
storage.insert_error(id);
}
pub(crate) fn unregister(&self, id: Id<T::Marker>) -> Option<T> {
let value = self.storage.write().remove(id);
// This needs to happen *after* removing it from the storage, to maintain the

View File

@ -1288,12 +1288,8 @@ impl Global {
) -> R {
profiling::scope!("Device::as_hal");
let hub = &self.hub;
let device = hub.devices.get(id).ok();
let hal_device = device
.as_ref()
.map(|device| device.raw())
.and_then(|device| device.as_any().downcast_ref());
let device = self.hub.devices.strict_get(id);
let hal_device = device.raw().as_any().downcast_ref();
hal_device_callback(hal_device)
}
@ -1308,14 +1304,9 @@ impl Global {
) -> R {
profiling::scope!("Device::fence_as_hal");
let hub = &self.hub;
if let Ok(device) = hub.devices.get(id) {
let fence = device.fence.read();
hal_fence_callback(fence.as_any().downcast_ref())
} else {
hal_fence_callback(None)
}
let device = self.hub.devices.strict_get(id);
let fence = device.fence.read();
hal_fence_callback(fence.as_any().downcast_ref())
}
/// # Safety

View File

@ -7550,10 +7550,4 @@ pub enum DeviceLostReason {
/// exactly once before it is dropped, which helps with managing the
/// memory owned by the callback.
ReplacedCallback = 3,
/// When setting the callback, but the device is already invalid
///
/// As above, when the callback is provided, wgpu guarantees that it
/// will eventually be called. If the device is already invalid, wgpu
/// will call the callback immediately, with this reason.
DeviceInvalid = 4,
}

View File

@ -464,12 +464,6 @@ impl Device {
)
}
/// Test-only function to make this device invalid.
#[doc(hidden)]
pub fn make_invalid(&self) {
DynContext::device_make_invalid(&*self.context, self.data.as_ref())
}
/// Create a [`PipelineCache`] with initial data
///
/// This can be passed to [`Device::create_compute_pipeline`]

View File

@ -2012,11 +2012,6 @@ impl crate::context::Context for ContextWebGpu {
Sendable(device_data.0.create_render_bundle_encoder(&mapped_desc))
}
#[doc(hidden)]
fn device_make_invalid(&self, _device_data: &Self::DeviceData) {
// Unimplemented
}
fn device_drop(&self, _device_data: &Self::DeviceData) {
// Device is dropped automatically
}

View File

@ -755,17 +755,11 @@ impl crate::Context for ContextWgpuCore {
}
fn device_features(&self, device_data: &Self::DeviceData) -> Features {
match self.0.device_features(device_data.id) {
Ok(features) => features,
Err(err) => self.handle_error_fatal(err, "Device::features"),
}
self.0.device_features(device_data.id)
}
fn device_limits(&self, device_data: &Self::DeviceData) -> Limits {
match self.0.device_limits(device_data.id) {
Ok(limits) => limits,
Err(err) => self.handle_error_fatal(err, "Device::limits"),
}
self.0.device_limits(device_data.id)
}
#[cfg_attr(
@ -1312,10 +1306,6 @@ impl crate::Context for ContextWgpuCore {
Err(e) => panic!("Error in Device::create_render_bundle_encoder: {e}"),
}
}
#[doc(hidden)]
fn device_make_invalid(&self, device_data: &Self::DeviceData) {
self.0.device_make_invalid(device_data.id);
}
#[cfg_attr(not(any(native, Emscripten)), allow(unused))]
fn device_drop(&self, device_data: &Self::DeviceData) {
#[cfg(any(native, Emscripten))]

View File

@ -195,8 +195,6 @@ pub trait Context: Debug + WasmNotSendSync + Sized {
device_data: &Self::DeviceData,
desc: &RenderBundleEncoderDescriptor<'_>,
) -> Self::RenderBundleEncoderData;
#[doc(hidden)]
fn device_make_invalid(&self, device_data: &Self::DeviceData);
fn device_drop(&self, device_data: &Self::DeviceData);
fn device_set_device_lost_callback(
&self,
@ -888,8 +886,6 @@ pub(crate) trait DynContext: Debug + WasmNotSendSync {
device_data: &crate::Data,
desc: &RenderBundleEncoderDescriptor<'_>,
) -> Box<crate::Data>;
#[doc(hidden)]
fn device_make_invalid(&self, device_data: &crate::Data);
fn device_drop(&self, device_data: &crate::Data);
fn device_set_device_lost_callback(
&self,
@ -1638,12 +1634,6 @@ where
Box::new(data) as _
}
#[doc(hidden)]
fn device_make_invalid(&self, device_data: &crate::Data) {
let device_data = downcast_ref(device_data);
Context::device_make_invalid(self, device_data)
}
fn device_drop(&self, device_data: &crate::Data) {
let device_data = downcast_ref(device_data);
Context::device_drop(self, device_data)