Skip to content

Commit

Permalink
Implement "tracker_asserts" feature in wgpu-core.
Browse files Browse the repository at this point in the history
Since `wgpu-core`'s public functions are supposed to validate their
parameters, the internal `track` module skips many of Rust's usual
run-time checks in release builds. However, some `wgpu-core` users
are happy to pay the performance cost in exchange for more safety.
The `"tracker_asserts"` feature causes the `track` module to perform
the same checks in release builds as it does in debug builds.
  • Loading branch information
jimblandy committed Jul 11, 2022
1 parent a108efe commit aa79dea
Show file tree
Hide file tree
Showing 8 changed files with 139 additions and 82 deletions.
2 changes: 1 addition & 1 deletion deno_webgpu/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ description = "WebGPU implementation for Deno"
deno_core = "0.135.0"
serde = { version = "1.0", features = ["derive"] }
tokio = { version = "1.17", features = ["full"] }
wgpu-core = { path = "../wgpu-core", features = ["trace", "replay", "serde"] }
wgpu-core = { path = "../wgpu-core", features = ["trace", "replay", "serde", "tracker_asserts"] }
wgpu-types = { path = "../wgpu-types", features = ["trace", "replay", "serde"] }
2 changes: 1 addition & 1 deletion player/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ features = ["replay"]
[dependencies.wgc]
path = "../wgpu-core"
package = "wgpu-core"
features = ["replay", "raw-window-handle"]
features = ["replay", "raw-window-handle", "tracker_asserts"]

[dev-dependencies]
serde = "1"
4 changes: 4 additions & 0 deletions wgpu-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ license = "MIT OR Apache-2.0"

[features]
default = []
# Apply run-time checks in the resource tracker, even in release builds.
# These are in addition to the validation carried out at public APIs
# in all builds.
tracker_asserts = []
angle = ["hal/gles"]
# Enable API tracing
trace = ["ron", "serde", "wgt/trace", "arrayvec/serde", "naga/serialize"]
Expand Down
49 changes: 49 additions & 0 deletions wgpu-core/src/track/assertions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
//! Macros for validation internal to the resource tracker.
//!
//! This module defines assertion macros that respect `wgpu-core`'s
//! `"tracker_asserts"` feature.
//!
//! Because `wgpu-core`'s public APIs validate their arguments in all
//! types of builds, for performance, the `track` module skips some of
//! Rust's usual run-time checks on its internal operations in release
//! builds. However, some `wgpu-core` applications have a strong
//! preference for robustness over performance. To accommodate them,
//! `wgpu-core`'s `"tracker_asserts"` feature enables that validation
//! in both debug and release builds.
#[cfg(feature = "tracker_asserts")]
macro_rules! tracker_assert {
( $( $arg:tt )* ) => {
assert!( $( $arg )* )
}
}

#[cfg(feature = "tracker_asserts")]
macro_rules! tracker_assert_eq {
( $( $arg:tt )* ) => {
assert_eq!( $( $arg )* )
}
}

#[cfg(feature = "tracker_asserts")]
macro_rules! tracker_assert_ne {
( $( $arg:tt )* ) => {
assert_ne!( $( $arg )* )
}
}

#[cfg(not(feature = "tracker_asserts"))]
#[macro_export]
macro_rules! tracker_assert {
( $( $arg:tt )* ) => {};
}

#[cfg(not(feature = "tracker_asserts"))]
macro_rules! tracker_assert_eq {
( $( $arg:tt )* ) => {};
}

#[cfg(not(feature = "tracker_asserts"))]
macro_rules! tracker_assert_ne {
( $( $arg:tt )* ) => {};
}
44 changes: 22 additions & 22 deletions wgpu-core/src/track/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,9 @@ impl<A: hub::HalApi> BufferUsageScope<A> {
}
}

fn debug_assert_in_bounds(&self, index: usize) {
debug_assert!(index < self.state.len());
self.metadata.debug_assert_in_bounds(index);
fn tracker_assert_in_bounds(&self, index: usize) {
tracker_assert!(index < self.state.len());
self.metadata.tracker_assert_in_bounds(index);
}

/// Sets the size of all the vectors inside the tracker.
Expand Down Expand Up @@ -179,8 +179,8 @@ impl<A: hub::HalApi> BufferUsageScope<A> {
}

for index in iterate_bitvec_indices(&scope.metadata.owned) {
self.debug_assert_in_bounds(index);
scope.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);
scope.tracker_assert_in_bounds(index);

unsafe {
insert_or_merge(
Expand Down Expand Up @@ -225,7 +225,7 @@ impl<A: hub::HalApi> BufferUsageScope<A> {

self.allow_index(index);

self.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);

unsafe {
insert_or_merge(
Expand Down Expand Up @@ -265,10 +265,10 @@ impl<A: hub::HalApi> BufferTracker<A> {
}
}

fn debug_assert_in_bounds(&self, index: usize) {
debug_assert!(index < self.start.len());
debug_assert!(index < self.end.len());
self.metadata.debug_assert_in_bounds(index);
fn tracker_assert_in_bounds(&self, index: usize) {
tracker_assert!(index < self.start.len());
tracker_assert!(index < self.end.len());
self.metadata.tracker_assert_in_bounds(index);
}

/// Sets the size of all the vectors inside the tracker.
Expand Down Expand Up @@ -311,7 +311,7 @@ impl<A: hub::HalApi> BufferTracker<A> {

self.allow_index(index);

self.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);

unsafe {
let currently_owned = self.metadata.owned.get(index).unwrap_unchecked();
Expand Down Expand Up @@ -356,7 +356,7 @@ impl<A: hub::HalApi> BufferTracker<A> {

self.allow_index(index);

self.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);

unsafe {
insert_or_barrier_update(
Expand All @@ -373,7 +373,7 @@ impl<A: hub::HalApi> BufferTracker<A> {
)
};

debug_assert!(self.temp.len() <= 1);
tracker_assert!(self.temp.len() <= 1);

Some((value, self.temp.pop()))
}
Expand All @@ -393,8 +393,8 @@ impl<A: hub::HalApi> BufferTracker<A> {
}

for index in iterate_bitvec_indices(&tracker.metadata.owned) {
self.debug_assert_in_bounds(index);
tracker.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);
tracker.tracker_assert_in_bounds(index);
unsafe {
insert_or_barrier_update(
None,
Expand Down Expand Up @@ -433,8 +433,8 @@ impl<A: hub::HalApi> BufferTracker<A> {
}

for index in iterate_bitvec_indices(&scope.metadata.owned) {
self.debug_assert_in_bounds(index);
scope.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);
scope.tracker_assert_in_bounds(index);
unsafe {
insert_or_barrier_update(
None,
Expand Down Expand Up @@ -488,7 +488,7 @@ impl<A: hub::HalApi> BufferTracker<A> {
let (index32, _, _) = id.0.unzip();
let index = index32 as usize;

scope.debug_assert_in_bounds(index);
scope.tracker_assert_in_bounds(index);

if !scope.metadata.owned.get(index).unwrap_unchecked() {
continue;
Expand Down Expand Up @@ -529,7 +529,7 @@ impl<A: hub::HalApi> BufferTracker<A> {
return false;
}

self.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);

unsafe {
if self.metadata.owned.get(index).unwrap_unchecked() {
Expand Down Expand Up @@ -569,7 +569,7 @@ impl BufferStateProvider<'_> {
match *self {
BufferStateProvider::Direct { state } => state,
BufferStateProvider::Indirect { state } => {
debug_assert!(index < state.len());
tracker_assert!(index < state.len());
*state.get_unchecked(index)
}
}
Expand Down Expand Up @@ -695,8 +695,8 @@ unsafe fn insert<A: hub::HalApi>(

// This should only ever happen with a wgpu bug, but let's just double
// check that resource states don't have any conflicts.
debug_assert_eq!(invalid_resource_state(new_start_state), false);
debug_assert_eq!(invalid_resource_state(new_end_state), false);
tracker_assert_eq!(invalid_resource_state(new_start_state), false);
tracker_assert_eq!(invalid_resource_state(new_end_state), false);

log::trace!("\tbuf {index}: insert {new_start_state:?}..{new_end_state:?}");

Expand Down
30 changes: 17 additions & 13 deletions wgpu-core/src/track/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@
* [`UsageScope`]: https://gpuweb.github.io/gpuweb/#programming-model-synchronization
!*/

#[macro_use]
mod assertions;

mod buffer;
mod range;
mod stateless;
Expand Down Expand Up @@ -143,13 +146,13 @@ impl PendingTransition<hal::TextureUses> {
let texture = tex.inner.as_raw().expect("Texture is destroyed");

// These showing up in a barrier is always a bug
debug_assert_ne!(self.usage.start, hal::TextureUses::UNKNOWN);
debug_assert_ne!(self.usage.end, hal::TextureUses::UNKNOWN);
tracker_assert_ne!(self.usage.start, hal::TextureUses::UNKNOWN);
tracker_assert_ne!(self.usage.end, hal::TextureUses::UNKNOWN);

let mip_count = self.selector.mips.end - self.selector.mips.start;
debug_assert_ne!(mip_count, 0);
tracker_assert_ne!(mip_count, 0);
let layer_count = self.selector.layers.end - self.selector.layers.start;
debug_assert_ne!(layer_count, 0);
tracker_assert_ne!(layer_count, 0);

hal::TextureBarrier {
texture,
Expand Down Expand Up @@ -351,12 +354,13 @@ impl<A: hub::HalApi> ResourceMetadata<A> {
/// sanity checks of the presence of a refcount.
///
/// In release mode this function is completely empty and is removed.
fn debug_assert_in_bounds(&self, index: usize) {
debug_assert!(index < self.owned.len());
debug_assert!(index < self.ref_counts.len());
debug_assert!(index < self.epochs.len());
#[cfg_attr(not(feature = "tracker_asserts"), allow(unused_variables))]
fn tracker_assert_in_bounds(&self, index: usize) {
tracker_assert!(index < self.owned.len());
tracker_assert!(index < self.ref_counts.len());
tracker_assert!(index < self.epochs.len());

debug_assert!(if self.owned.get(index).unwrap() {
tracker_assert!(if self.owned.get(index).unwrap() {
self.ref_counts[index].is_some()
} else {
true
Expand All @@ -373,7 +377,7 @@ impl<A: hub::HalApi> ResourceMetadata<A> {
/// Returns ids for all resources we own.
fn used<Id: TypedId>(&self) -> impl Iterator<Item = id::Valid<Id>> + '_ {
if !self.owned.is_empty() {
self.debug_assert_in_bounds(self.owned.len() - 1)
self.tracker_assert_in_bounds(self.owned.len() - 1)
};
iterate_bitvec_indices(&self.owned).map(move |index| {
let epoch = unsafe { *self.epochs.get_unchecked(index) };
Expand Down Expand Up @@ -418,7 +422,7 @@ impl<A: hub::HalApi> ResourceMetadataProvider<'_, A> {
(epoch, ref_count.into_owned())
}
ResourceMetadataProvider::Indirect { metadata } => {
metadata.debug_assert_in_bounds(index);
metadata.tracker_assert_in_bounds(index);
(
*metadata.epochs.get_unchecked(index),
metadata
Expand All @@ -429,7 +433,7 @@ impl<A: hub::HalApi> ResourceMetadataProvider<'_, A> {
)
}
ResourceMetadataProvider::Resource { epoch } => {
debug_assert!(life_guard.is_some());
tracker_assert!(life_guard.is_some());
(epoch, life_guard.unwrap_unchecked().add_ref())
}
}
Expand All @@ -445,7 +449,7 @@ impl<A: hub::HalApi> ResourceMetadataProvider<'_, A> {
ResourceMetadataProvider::Direct { epoch, .. }
| ResourceMetadataProvider::Resource { epoch, .. } => epoch,
ResourceMetadataProvider::Indirect { metadata } => {
metadata.debug_assert_in_bounds(index);
metadata.tracker_assert_in_bounds(index);
*metadata.epochs.get_unchecked(index)
}
}
Expand Down
14 changes: 7 additions & 7 deletions wgpu-core/src/track/stateless.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ impl<A: hub::HalApi, T: hub::Resource, Id: TypedId> StatelessTracker<A, T, Id> {
}
}

fn debug_assert_in_bounds(&self, index: usize) {
self.metadata.debug_assert_in_bounds(index);
fn tracker_assert_in_bounds(&self, index: usize) {
self.metadata.tracker_assert_in_bounds(index);
}

/// Sets the size of all the vectors inside the tracker.
Expand Down Expand Up @@ -106,7 +106,7 @@ impl<A: hub::HalApi, T: hub::Resource, Id: TypedId> StatelessTracker<A, T, Id> {

self.allow_index(index);

self.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);

unsafe {
*self.metadata.epochs.get_unchecked_mut(index) = epoch;
Expand All @@ -127,7 +127,7 @@ impl<A: hub::HalApi, T: hub::Resource, Id: TypedId> StatelessTracker<A, T, Id> {

self.allow_index(index);

self.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);

unsafe {
*self.metadata.epochs.get_unchecked_mut(index) = epoch;
Expand All @@ -149,8 +149,8 @@ impl<A: hub::HalApi, T: hub::Resource, Id: TypedId> StatelessTracker<A, T, Id> {
}

for index in iterate_bitvec_indices(&other.metadata.owned) {
self.debug_assert_in_bounds(index);
other.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);
other.tracker_assert_in_bounds(index);
unsafe {
let previously_owned = self.metadata.owned.get(index).unwrap_unchecked();

Expand Down Expand Up @@ -187,7 +187,7 @@ impl<A: hub::HalApi, T: hub::Resource, Id: TypedId> StatelessTracker<A, T, Id> {
return false;
}

self.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);

unsafe {
if self.metadata.owned.get(index).unwrap_unchecked() {
Expand Down
Loading

0 comments on commit aa79dea

Please sign in to comment.