From 16d41cfaf3a45837a33d597ee84c8458ce0523d3 Mon Sep 17 00:00:00 2001 From: Kevin Reid Date: Thu, 6 Mar 2025 11:07:38 -0800 Subject: [PATCH] Make `std` usage optional for `wgpu-core`. (#7279) * Make `wgpu_core::snatch::LockTrace` fully instead of partially conditional. Now, when `cfg(not(debug_assertions))`, there is no `SNATCH_LOCK_TRACE` thread local, and `LockTrace` has no data fields. * Make `std` usage optional for `wgpu-core`. Adds a `std` feature, enabled by default, to `wgpu-core`. When that feature is disabled, the following functionality is not available: * `Send + Sync` for resources. * `trace` feature. * `observe_locks` feature. * Snatch lock recursive locking assertion. --- wgpu-core/Cargo.toml | 13 +++- wgpu-core/build.rs | 9 ++- wgpu-core/src/binding_model.rs | 3 +- wgpu-core/src/device/resource.rs | 9 ++- wgpu-core/src/lib.rs | 39 ++++++----- wgpu-core/src/lock/mod.rs | 1 + wgpu-core/src/snatch.rs | 111 ++++++++++++++++--------------- 7 files changed, 105 insertions(+), 80 deletions(-) diff --git a/wgpu-core/Cargo.toml b/wgpu-core/Cargo.toml index b5b56828d..d5109c277 100644 --- a/wgpu-core/Cargo.toml +++ b/wgpu-core/Cargo.toml @@ -37,6 +37,11 @@ unexpected_cfgs = { level = "warn", check-cfg = ['cfg(wgpu_validate_locks)'] } [features] #! See docuemntation for the `wgpu` crate for more in-depth information on these features. +# TODO(https://github.com/gfx-rs/wgpu/issues/6826): "std" is a default feature for +# compatibility with prior behavior only, and should be removed once we know how +# wgpu-core’s dependents want to handle no_std. +default = ["std"] + #! ### Logging Configuration # -------------------------------------------------------------------- @@ -62,7 +67,7 @@ strict_asserts = ["wgpu-types/strict_asserts"] # -------------------------------------------------------------------- ## Enable lock order observation. -observe_locks = ["dep:ron", "serde/serde_derive"] +observe_locks = ["std", "dep:ron", "serde/serde_derive"] #! ### Serialization # -------------------------------------------------------------------- @@ -71,7 +76,7 @@ observe_locks = ["dep:ron", "serde/serde_derive"] serde = ["dep:serde", "wgpu-types/serde", "arrayvec/serde", "hashbrown/serde"] ## Enable API tracing. -trace = ["dep:ron", "serde", "naga/serialize"] +trace = ["serde", "std", "dep:ron", "naga/serialize"] ## Enable API replaying replay = ["serde", "naga/deserialize"] @@ -107,6 +112,10 @@ fragile-send-sync-non-atomic-wasm = [ "wgpu-hal/fragile-send-sync-non-atomic-wasm", ] +## Enable certain items to be `Send` and `Sync` when they would not otherwise be. +## Also enables backtraces in some error cases when also under cfg(debug_assertions). +std = [] + #! ### External libraries # -------------------------------------------------------------------- #! The following features facilitate integration with third-party supporting libraries. diff --git a/wgpu-core/build.rs b/wgpu-core/build.rs index fde109a7f..a01f5afe3 100644 --- a/wgpu-core/build.rs +++ b/wgpu-core/build.rs @@ -1,9 +1,12 @@ fn main() { cfg_aliases::cfg_aliases! { windows_linux_android: { any(windows, target_os = "linux", target_os = "android") }, - send_sync: { any( - not(target_arch = "wasm32"), - all(feature = "fragile-send-sync-non-atomic-wasm", not(target_feature = "atomics")) + send_sync: { all( + feature = "std", + any( + not(target_arch = "wasm32"), + all(feature = "fragile-send-sync-non-atomic-wasm", not(target_feature = "atomics")) + ) ) }, dx12: { all(target_os = "windows", feature = "dx12") }, webgl: { all(target_arch = "wasm32", not(target_os = "emscripten"), feature = "webgl") }, diff --git a/wgpu-core/src/binding_model.rs b/wgpu-core/src/binding_model.rs index 5f6d52e00..76dbdfc55 100644 --- a/wgpu-core/src/binding_model.rs +++ b/wgpu-core/src/binding_model.rs @@ -6,7 +6,6 @@ use alloc::{ vec::Vec, }; use core::{fmt, mem::ManuallyDrop, ops::Range}; -use std::sync::OnceLock; use arrayvec::ArrayVec; use thiserror::Error; @@ -602,7 +601,7 @@ pub struct BindGroupLayout { /// We cannot unconditionally remove from the pool, as BGLs that don't come from the pool /// (derived BGLs) must not be removed. pub(crate) origin: bgl::Origin, - pub(crate) exclusive_pipeline: OnceLock, + pub(crate) exclusive_pipeline: crate::OnceCellOrLock, #[allow(unused)] pub(crate) binding_count_validator: BindingTypeMaxCountValidator, /// The `label` from the descriptor used to create the resource. diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index b61bf1442..bd19a20bf 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -11,7 +11,6 @@ use core::{ num::NonZeroU32, sync::atomic::{AtomicBool, Ordering}, }; -use std::sync::OnceLock; use arrayvec::ArrayVec; use bitflags::Flags; @@ -49,7 +48,7 @@ use crate::{ track::{BindGroupStates, DeviceTracker, TrackerIndexAllocators, UsageScope, UsageScopePool}, validation::{self, validate_color_attachment_bytes_per_sample}, weak_vec::WeakVec, - FastHashMap, LabelHelpers, + FastHashMap, LabelHelpers, OnceCellOrLock, }; use super::{ @@ -67,7 +66,7 @@ use portable_atomic::AtomicU64; pub struct Device { raw: Box, pub(crate) adapter: Arc, - pub(crate) queue: OnceLock>, + pub(crate) queue: OnceCellOrLock>, pub(crate) zero_buffer: ManuallyDrop>, /// The `label` from the descriptor used to create the resource. label: String, @@ -257,7 +256,7 @@ impl Device { Ok(Self { raw: raw_device, adapter: adapter.clone(), - queue: OnceLock::new(), + queue: OnceCellOrLock::new(), zero_buffer: ManuallyDrop::new(zero_buffer), label: desc.label.to_string(), command_allocator, @@ -1986,7 +1985,7 @@ impl Device { device: self.clone(), entries: entry_map, origin, - exclusive_pipeline: OnceLock::new(), + exclusive_pipeline: OnceCellOrLock::new(), binding_count_validator: count_validator, label: label.to_string(), }; diff --git a/wgpu-core/src/lib.rs b/wgpu-core/src/lib.rs index db3fcde38..cdae6b927 100644 --- a/wgpu-core/src/lib.rs +++ b/wgpu-core/src/lib.rs @@ -63,7 +63,7 @@ #![cfg_attr(not(send_sync), allow(clippy::arc_with_non_send_sync))] extern crate alloc; -// TODO(https://github.com/gfx-rs/wgpu/issues/6826): this should be optional +#[cfg(feature = "std")] extern crate std; extern crate wgpu_hal as hal; extern crate wgpu_types as wgt; @@ -111,7 +111,6 @@ use alloc::{ borrow::{Cow, ToOwned as _}, string::String, }; -use std::os::raw::c_char; pub(crate) use hash_utils::*; @@ -123,7 +122,7 @@ pub type SubmissionIndex = hal::FenceValue; type Index = u32; type Epoch = u32; -pub type RawString = *const c_char; +pub type RawString = *const core::ffi::c_char; pub type Label<'a> = Option>; trait LabelHelpers<'a> { @@ -227,17 +226,27 @@ pub(crate) fn get_greatest_common_divisor(mut a: u32, mut b: u32) -> u32 { } } -#[test] -fn test_lcd() { - assert_eq!(get_lowest_common_denom(2, 2), 2); - assert_eq!(get_lowest_common_denom(2, 3), 6); - assert_eq!(get_lowest_common_denom(6, 4), 12); -} +#[cfg(not(feature = "std"))] +use core::cell::OnceCell as OnceCellOrLock; +#[cfg(feature = "std")] +use std::sync::OnceLock as OnceCellOrLock; -#[test] -fn test_gcd() { - assert_eq!(get_greatest_common_divisor(5, 1), 1); - assert_eq!(get_greatest_common_divisor(4, 2), 2); - assert_eq!(get_greatest_common_divisor(6, 4), 2); - assert_eq!(get_greatest_common_divisor(7, 7), 7); +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_lcd() { + assert_eq!(get_lowest_common_denom(2, 2), 2); + assert_eq!(get_lowest_common_denom(2, 3), 6); + assert_eq!(get_lowest_common_denom(6, 4), 12); + } + + #[test] + fn test_gcd() { + assert_eq!(get_greatest_common_divisor(5, 1), 1); + assert_eq!(get_greatest_common_divisor(4, 2), 2); + assert_eq!(get_greatest_common_divisor(6, 4), 2); + assert_eq!(get_greatest_common_divisor(7, 7), 7); + } } diff --git a/wgpu-core/src/lock/mod.rs b/wgpu-core/src/lock/mod.rs index 2927bf3aa..612dec87d 100644 --- a/wgpu-core/src/lock/mod.rs +++ b/wgpu-core/src/lock/mod.rs @@ -33,6 +33,7 @@ pub mod rank; +#[cfg(feature = "std")] // requires thread-locals to work #[cfg_attr(not(wgpu_validate_locks), allow(dead_code))] mod ranked; diff --git a/wgpu-core/src/snatch.rs b/wgpu-core/src/snatch.rs index 8c2c7bd9b..525598271 100644 --- a/wgpu-core/src/snatch.rs +++ b/wgpu-core/src/snatch.rs @@ -1,18 +1,11 @@ -#![allow(unused)] - -use core::{ - cell::{Cell, RefCell, UnsafeCell}, - fmt, - panic::{self, Location}, -}; -use std::{backtrace::Backtrace, thread}; +use core::{cell::UnsafeCell, fmt}; use crate::lock::{rank, RwLock, RwLockReadGuard, RwLockWriteGuard}; /// A guard that provides read access to snatchable data. -pub struct SnatchGuard<'a>(RwLockReadGuard<'a, ()>); +pub struct SnatchGuard<'a>(#[expect(dead_code)] RwLockReadGuard<'a, ()>); /// A guard that allows snatching the snatchable data. -pub struct ExclusiveSnatchGuard<'a>(RwLockWriteGuard<'a, ()>); +pub struct ExclusiveSnatchGuard<'a>(#[expect(dead_code)] RwLockWriteGuard<'a, ()>); /// A value that is mostly immutable but can be "snatched" if we need to destroy /// it early. @@ -31,6 +24,7 @@ impl Snatchable { } } + #[allow(dead_code)] pub fn empty() -> Self { Snatchable { value: UnsafeCell::new(None), @@ -66,58 +60,69 @@ impl fmt::Debug for Snatchable { unsafe impl Sync for Snatchable {} -struct LockTrace { - purpose: &'static str, - caller: &'static Location<'static>, - backtrace: Backtrace, -} +use trace::LockTrace; +#[cfg(all(debug_assertions, feature = "std"))] +mod trace { + use core::{cell::Cell, fmt, panic::Location}; + use std::{backtrace::Backtrace, thread}; -impl fmt::Display for LockTrace { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "a {} lock at {}\n{}", - self.purpose, self.caller, self.backtrace - ) + pub(super) struct LockTrace { + purpose: &'static str, + caller: &'static Location<'static>, + backtrace: Backtrace, } -} -#[cfg(debug_assertions)] -impl LockTrace { - #[track_caller] - fn enter(purpose: &'static str) { - let new = LockTrace { - purpose, - caller: Location::caller(), - backtrace: Backtrace::capture(), - }; - - if let Some(prev) = SNATCH_LOCK_TRACE.take() { - let current = thread::current(); - let name = current.name().unwrap_or(""); - panic!( - "thread '{name}' attempted to acquire a snatch lock recursively.\n\ - - Currently trying to acquire {new}\n\ - - Previously acquired {prev}", - ); - } else { - SNATCH_LOCK_TRACE.set(Some(new)); + impl fmt::Display for LockTrace { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "a {} lock at {}\n{}", + self.purpose, self.caller, self.backtrace + ) } } - fn exit() { - SNATCH_LOCK_TRACE.take(); + impl LockTrace { + #[track_caller] + pub(super) fn enter(purpose: &'static str) { + let new = LockTrace { + purpose, + caller: Location::caller(), + backtrace: Backtrace::capture(), + }; + + if let Some(prev) = SNATCH_LOCK_TRACE.take() { + let current = thread::current(); + let name = current.name().unwrap_or(""); + panic!( + "thread '{name}' attempted to acquire a snatch lock recursively.\n\ + - Currently trying to acquire {new}\n\ + - Previously acquired {prev}", + ); + } else { + SNATCH_LOCK_TRACE.set(Some(new)); + } + } + + pub(super) fn exit() { + SNATCH_LOCK_TRACE.take(); + } + } + + std::thread_local! { + static SNATCH_LOCK_TRACE: Cell> = const { Cell::new(None) }; } } +#[cfg(not(all(debug_assertions, feature = "std")))] +mod trace { + pub(super) struct LockTrace { + _private: (), + } -#[cfg(not(debug_assertions))] -impl LockTrace { - fn enter(purpose: &'static str) {} - fn exit() {} -} - -std::thread_local! { - static SNATCH_LOCK_TRACE: Cell> = const { Cell::new(None) }; + impl LockTrace { + pub(super) fn enter(_purpose: &'static str) {} + pub(super) fn exit() {} + } } /// A Device-global lock for all snatchable data.