From 76e9bf9c9956aa2d3d860617ffba57912058f82a Mon Sep 17 00:00:00 2001 From: Zachary Harrold Date: Tue, 25 Feb 2025 07:52:46 +1100 Subject: [PATCH] Automatically enable `portable-atomic` when required (#17570) # Objective - Contributes to #15460 - Reduce quantity and complexity of feature gates across Bevy ## Solution - Used `target_has_atomic` configuration variable to automatically detect impartial atomic support and automatically switch to `portable-atomic` over the standard library on an as-required basis. ## Testing - CI ## Notes To explain the technique employed here, consider getting `Arc` either from `alloc::sync` _or_ `portable-atomic-util`. First, we can inspect the `alloc` crate to see that you only have access to `Arc` _if_ `target_has_atomic = "ptr"`. We add a target dependency for this particular configuration _inverted_: ```toml [target.'cfg(not(target_has_atomic = "ptr"))'.dependencies] portable-atomic-util = { version = "0.2.4", default-features = false } ``` This ensures we only have the dependency when it is needed, and it is entirely excluded from the dependency graph when it is not. Next, we adjust our configuration flags to instead of checking for `feature = "portable-atomic"` to instead check for `target_has_atomic = "ptr"`: ```rust // `alloc` feature flag hidden for brevity #[cfg(not(target_has_atomic = "ptr"))] use portable_atomic_util as arc; #[cfg(target_has_atomic = "ptr")] use alloc::sync as arc; pub use arc::{Arc, Weak}; ``` The benefits of this technique are three-fold: 1. For platforms without full atomic support, the functionality is enabled automatically. 2. For platforms with atomic support, the dependency is never included, even if a feature was enabled using `--all-features` (for example) 3. The `portable-atomic` feature no longer needs to virally spread to all user-facing crates, it's instead something handled within `bevy_platform_support` (with some extras where other dependencies also need their features enabled). --- crates/bevy_a11y/Cargo.toml | 9 ---- crates/bevy_app/Cargo.toml | 9 ---- crates/bevy_app/src/task_pool_plugin.rs | 8 ---- crates/bevy_diagnostic/Cargo.toml | 11 ----- crates/bevy_ecs/Cargo.toml | 14 +++---- crates/bevy_ecs/src/component.rs | 6 +-- crates/bevy_input/Cargo.toml | 9 ---- crates/bevy_input_focus/Cargo.toml | 9 ---- crates/bevy_platform_support/Cargo.toml | 31 +++++++------- .../bevy_platform_support/src/sync/atomic.rs | 42 +++++++++++++++---- crates/bevy_platform_support/src/sync/mod.rs | 4 +- crates/bevy_reflect/Cargo.toml | 7 ---- .../bevy_reflect/src/func/dynamic_function.rs | 2 +- crates/bevy_reflect/src/generics.rs | 2 +- crates/bevy_reflect/src/impls/std.rs | 5 --- crates/bevy_reflect/src/serde/mod.rs | 2 +- crates/bevy_state/Cargo.toml | 10 ----- crates/bevy_tasks/Cargo.toml | 13 +++--- crates/bevy_tasks/src/task_pool.rs | 40 +++++++++++------- crates/bevy_time/Cargo.toml | 9 ---- crates/bevy_utils/Cargo.toml | 4 -- 21 files changed, 94 insertions(+), 152 deletions(-) diff --git a/crates/bevy_a11y/Cargo.toml b/crates/bevy_a11y/Cargo.toml index 4da1040620cf1..3a6ece0652868 100644 --- a/crates/bevy_a11y/Cargo.toml +++ b/crates/bevy_a11y/Cargo.toml @@ -50,15 +50,6 @@ critical-section = [ "bevy_input_focus/critical-section", ] -## `portable-atomic` provides additional platform support for atomic types and -## operations, even on targets without native support. -portable-atomic = [ - "bevy_app/portable-atomic", - "bevy_ecs/portable-atomic", - "bevy_reflect?/portable-atomic", - "bevy_input_focus/portable-atomic", -] - ## Uses the `libm` maths library instead of the one provided in `std` and `core`. libm = ["bevy_input_focus/libm"] diff --git a/crates/bevy_app/Cargo.toml b/crates/bevy_app/Cargo.toml index d62428abdaa4e..c19a3329d3434 100644 --- a/crates/bevy_app/Cargo.toml +++ b/crates/bevy_app/Cargo.toml @@ -60,15 +60,6 @@ critical-section = [ "bevy_reflect?/critical-section", ] -## `portable-atomic` provides additional platform support for atomic types and -## operations, even on targets without native support. -portable-atomic = [ - "bevy_tasks?/portable-atomic", - "bevy_ecs/portable-atomic", - "bevy_platform_support/portable-atomic", - "bevy_reflect?/portable-atomic", -] - [dependencies] # bevy bevy_derive = { path = "../bevy_derive", version = "0.16.0-dev" } diff --git a/crates/bevy_app/src/task_pool_plugin.rs b/crates/bevy_app/src/task_pool_plugin.rs index d2146d9a65a46..174bca105b6b5 100644 --- a/crates/bevy_app/src/task_pool_plugin.rs +++ b/crates/bevy_app/src/task_pool_plugin.rs @@ -1,11 +1,3 @@ -#![cfg_attr( - feature = "portable-atomic", - expect( - clippy::redundant_closure, - reason = "bevy_platform_support::sync::Arc has subtly different implicit behavior" - ) -)] - use crate::{App, Plugin}; use alloc::string::ToString; diff --git a/crates/bevy_diagnostic/Cargo.toml b/crates/bevy_diagnostic/Cargo.toml index 80afd7022f038..be8f7cddcccde 100644 --- a/crates/bevy_diagnostic/Cargo.toml +++ b/crates/bevy_diagnostic/Cargo.toml @@ -54,17 +54,6 @@ critical-section = [ "bevy_tasks?/critical-section", ] -## `portable-atomic` provides additional platform support for atomic types and -## operations, even on targets without native support. -portable-atomic = [ - "bevy_ecs/portable-atomic", - "bevy_app/portable-atomic", - "bevy_platform_support/portable-atomic", - "bevy_time/portable-atomic", - "bevy_utils/portable-atomic", - "bevy_tasks?/portable-atomic", -] - [dependencies] # bevy bevy_app = { path = "../bevy_app", version = "0.16.0-dev", default-features = false } diff --git a/crates/bevy_ecs/Cargo.toml b/crates/bevy_ecs/Cargo.toml index 89a13c5e9a162..e42b2b62a0650 100644 --- a/crates/bevy_ecs/Cargo.toml +++ b/crates/bevy_ecs/Cargo.toml @@ -92,15 +92,6 @@ critical-section = [ "bevy_reflect?/critical-section", ] -## `portable-atomic` provides additional platform support for atomic types and -## operations, even on targets without native support. -portable-atomic = [ - "bevy_tasks?/portable-atomic", - "bevy_platform_support/portable-atomic", - "concurrent-queue/portable-atomic", - "bevy_reflect?/portable-atomic", -] - [dependencies] bevy_ptr = { path = "../bevy_ptr", version = "0.16.0-dev" } bevy_reflect = { path = "../bevy_reflect", version = "0.16.0-dev", features = [ @@ -139,6 +130,11 @@ tracing = { version = "0.1", default-features = false, optional = true } log = { version = "0.4", default-features = false } bumpalo = "3" +[target.'cfg(not(all(target_has_atomic = "8", target_has_atomic = "16", target_has_atomic = "32", target_has_atomic = "64", target_has_atomic = "ptr")))'.dependencies] +concurrent-queue = { version = "2.5.0", default-features = false, features = [ + "portable-atomic", +] } + [dev-dependencies] rand = "0.8" static_assertions = "1.1.0" diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 0eaa329c61a30..e2712dcb7b2fb 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -2060,7 +2060,7 @@ impl RequiredComponents { // // This would be resolved by https://github.com/rust-lang/rust/issues/123430 - #[cfg(feature = "portable-atomic")] + #[cfg(not(target_has_atomic = "ptr"))] use alloc::boxed::Box; type Constructor = dyn for<'a, 'b> Fn( @@ -2072,10 +2072,10 @@ impl RequiredComponents { MaybeLocation, ); - #[cfg(feature = "portable-atomic")] + #[cfg(not(target_has_atomic = "ptr"))] type Intermediate = Box; - #[cfg(not(feature = "portable-atomic"))] + #[cfg(target_has_atomic = "ptr")] type Intermediate = Arc; let boxed: Intermediate = Intermediate::new( diff --git a/crates/bevy_input/Cargo.toml b/crates/bevy_input/Cargo.toml index 2b09ed5e21136..602d1be8eac76 100644 --- a/crates/bevy_input/Cargo.toml +++ b/crates/bevy_input/Cargo.toml @@ -56,15 +56,6 @@ critical-section = [ "bevy_platform_support/critical-section", ] -## `portable-atomic` provides additional platform support for atomic types and -## operations, even on targets without native support. -portable-atomic = [ - "bevy_app/portable-atomic", - "bevy_ecs/portable-atomic", - "bevy_reflect?/portable-atomic", - "bevy_platform_support/portable-atomic", -] - ## Uses the `libm` maths library instead of the one provided in `std` and `core`. libm = ["bevy_math/libm"] diff --git a/crates/bevy_input_focus/Cargo.toml b/crates/bevy_input_focus/Cargo.toml index 72a159cf03063..0b2ca538307e7 100644 --- a/crates/bevy_input_focus/Cargo.toml +++ b/crates/bevy_input_focus/Cargo.toml @@ -55,15 +55,6 @@ critical-section = [ "bevy_input/critical-section", ] -## `portable-atomic` provides additional platform support for atomic types and -## operations, even on targets without native support. -portable-atomic = [ - "bevy_app/portable-atomic", - "bevy_ecs/portable-atomic", - "bevy_reflect?/portable-atomic", - "bevy_input/portable-atomic", -] - ## Uses the `libm` maths library instead of the one provided in `std` and `core`. libm = ["bevy_math/libm", "bevy_window/libm"] diff --git a/crates/bevy_platform_support/Cargo.toml b/crates/bevy_platform_support/Cargo.toml index cd45267b5faaf..7b0300b821f3b 100644 --- a/crates/bevy_platform_support/Cargo.toml +++ b/crates/bevy_platform_support/Cargo.toml @@ -24,32 +24,20 @@ serialize = ["hashbrown/serde"] std = [ "alloc", "critical-section?/std", - "portable-atomic?/std", - "portable-atomic-util?/std", + "portable-atomic/std", + "portable-atomic-util/std", "spin/std", "foldhash/std", ] -alloc = ["portable-atomic-util?/alloc", "dep:hashbrown"] +alloc = ["portable-atomic-util/alloc", "dep:hashbrown"] ## `critical-section` provides the building blocks for synchronization primitives ## on all platforms, including `no_std`. -critical-section = ["dep:critical-section", "portable-atomic?/critical-section"] - -## `portable-atomic` provides additional platform support for atomic types and -## operations, even on targets without native support. -portable-atomic = [ - "dep:portable-atomic", - "dep:portable-atomic-util", - "spin/portable_atomic", -] +critical-section = ["dep:critical-section", "portable-atomic/critical-section"] [dependencies] critical-section = { version = "1.2.0", default-features = false, optional = true } -portable-atomic = { version = "1", default-features = false, features = [ - "fallback", -], optional = true } -portable-atomic-util = { version = "0.2.4", default-features = false, optional = true } spin = { version = "0.9.8", default-features = false, features = [ "mutex", "spin_mutex", @@ -68,6 +56,17 @@ hashbrown = { version = "0.15.1", features = [ web-time = { version = "1.1", default-features = false } getrandom = { version = "0.2.0", default-features = false, features = ["js"] } +[target.'cfg(not(all(target_has_atomic = "8", target_has_atomic = "16", target_has_atomic = "32", target_has_atomic = "64", target_has_atomic = "ptr")))'.dependencies] +portable-atomic = { version = "1", default-features = false, features = [ + "fallback", +] } +spin = { version = "0.9.8", default-features = false, features = [ + "portable_atomic", +] } + +[target.'cfg(not(target_has_atomic = "ptr"))'.dependencies] +portable-atomic-util = { version = "0.2.4", default-features = false } + [lints] workspace = true diff --git a/crates/bevy_platform_support/src/sync/atomic.rs b/crates/bevy_platform_support/src/sync/atomic.rs index 9e8eadb3df8de..65211482a6cfc 100644 --- a/crates/bevy_platform_support/src/sync/atomic.rs +++ b/crates/bevy_platform_support/src/sync/atomic.rs @@ -5,13 +5,39 @@ //! Using these types will ensure the correct atomic provider is used without the need for //! feature gates in your own code. -pub use atomic::{ - AtomicBool, AtomicI16, AtomicI32, AtomicI64, AtomicI8, AtomicIsize, AtomicPtr, AtomicU16, - AtomicU32, AtomicU64, AtomicU8, AtomicUsize, Ordering, -}; +pub use atomic_16::{AtomicI16, AtomicU16}; +pub use atomic_32::{AtomicI32, AtomicU32}; +pub use atomic_64::{AtomicI64, AtomicU64}; +pub use atomic_8::{AtomicBool, AtomicI8, AtomicU8}; +pub use atomic_ptr::{AtomicIsize, AtomicPtr, AtomicUsize}; +pub use core::sync::atomic::Ordering; -#[cfg(not(feature = "portable-atomic"))] -use core::sync::atomic; +#[cfg(target_has_atomic = "8")] +use core::sync::atomic as atomic_8; -#[cfg(feature = "portable-atomic")] -use portable_atomic as atomic; +#[cfg(not(target_has_atomic = "8"))] +use portable_atomic as atomic_8; + +#[cfg(target_has_atomic = "16")] +use core::sync::atomic as atomic_16; + +#[cfg(not(target_has_atomic = "16"))] +use portable_atomic as atomic_16; + +#[cfg(target_has_atomic = "32")] +use core::sync::atomic as atomic_32; + +#[cfg(not(target_has_atomic = "32"))] +use portable_atomic as atomic_32; + +#[cfg(target_has_atomic = "64")] +use core::sync::atomic as atomic_64; + +#[cfg(not(target_has_atomic = "64"))] +use portable_atomic as atomic_64; + +#[cfg(target_has_atomic = "ptr")] +use core::sync::atomic as atomic_ptr; + +#[cfg(not(target_has_atomic = "ptr"))] +use portable_atomic as atomic_ptr; diff --git a/crates/bevy_platform_support/src/sync/mod.rs b/crates/bevy_platform_support/src/sync/mod.rs index edb0217262597..8fb7a2fbffaae 100644 --- a/crates/bevy_platform_support/src/sync/mod.rs +++ b/crates/bevy_platform_support/src/sync/mod.rs @@ -26,8 +26,8 @@ mod once; mod poison; mod rwlock; -#[cfg(all(feature = "alloc", feature = "portable-atomic"))] +#[cfg(all(feature = "alloc", not(target_has_atomic = "ptr")))] use portable_atomic_util as arc; -#[cfg(all(feature = "alloc", not(feature = "portable-atomic")))] +#[cfg(all(feature = "alloc", target_has_atomic = "ptr"))] use alloc::sync as arc; diff --git a/crates/bevy_reflect/Cargo.toml b/crates/bevy_reflect/Cargo.toml index e2bfbd46ddcd7..ada4237cc95d5 100644 --- a/crates/bevy_reflect/Cargo.toml +++ b/crates/bevy_reflect/Cargo.toml @@ -71,13 +71,6 @@ critical-section = [ "bevy_utils/critical-section", ] -## `portable-atomic` provides additional platform support for atomic types and -## operations, even on targets without native support. -portable-atomic = [ - "bevy_platform_support/portable-atomic", - "bevy_utils/portable-atomic", -] - [dependencies] # bevy bevy_reflect_derive = { path = "derive", version = "0.16.0-dev" } diff --git a/crates/bevy_reflect/src/func/dynamic_function.rs b/crates/bevy_reflect/src/func/dynamic_function.rs index c090442ce483f..6d1c9776a9f7b 100644 --- a/crates/bevy_reflect/src/func/dynamic_function.rs +++ b/crates/bevy_reflect/src/func/dynamic_function.rs @@ -94,7 +94,7 @@ impl<'env> DynamicFunction<'env> { ) -> Self { let arc = Arc::new(func); - #[cfg(feature = "portable-atomic")] + #[cfg(not(target_has_atomic = "ptr"))] #[expect( unsafe_code, reason = "unsized coercion is an unstable feature for non-std types" diff --git a/crates/bevy_reflect/src/generics.rs b/crates/bevy_reflect/src/generics.rs index 32b13100f552f..dfe21c6ec0843 100644 --- a/crates/bevy_reflect/src/generics.rs +++ b/crates/bevy_reflect/src/generics.rs @@ -183,7 +183,7 @@ impl ConstParamInfo { pub fn with_default(mut self, default: T) -> Self { let arc = Arc::new(default); - #[cfg(feature = "portable-atomic")] + #[cfg(not(target_has_atomic = "ptr"))] #[expect( unsafe_code, reason = "unsized coercion is an unstable feature for non-std types" diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index 447e480f14273..119b66e3ed6d0 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -217,11 +217,6 @@ impl_reflect_opaque!(::core::num::Wrapping()); impl_reflect_opaque!(::core::num::Saturating()); impl_reflect_opaque!(::bevy_platform_support::sync::Arc); -// We check despite `portable-atomic` being enabled, if the standard library `Arc` is -// also available, and implement Reflect for it. -#[cfg(all(feature = "portable-atomic", target_has_atomic = "ptr"))] -impl_reflect_opaque!(::alloc::sync::Arc); - // `Serialize` and `Deserialize` only for platforms supported by serde: // https://github.com/serde-rs/serde/blob/3ffb86fc70efd3d329519e2dddfa306cc04f167c/serde/src/de/impls.rs#L1732 #[cfg(all(any(unix, windows), feature = "std"))] diff --git a/crates/bevy_reflect/src/serde/mod.rs b/crates/bevy_reflect/src/serde/mod.rs index c31d97456679f..cf682edbeada2 100644 --- a/crates/bevy_reflect/src/serde/mod.rs +++ b/crates/bevy_reflect/src/serde/mod.rs @@ -340,7 +340,7 @@ mod tests { fn create_arc_dyn_enemy(enemy: T) -> Arc { let arc = Arc::new(enemy); - #[cfg(feature = "portable-atomic")] + #[cfg(not(target_has_atomic = "ptr"))] #[expect( unsafe_code, reason = "unsized coercion is an unstable feature for non-std types" diff --git a/crates/bevy_state/Cargo.toml b/crates/bevy_state/Cargo.toml index 9425961023faa..3b205fa69c092 100644 --- a/crates/bevy_state/Cargo.toml +++ b/crates/bevy_state/Cargo.toml @@ -46,16 +46,6 @@ critical-section = [ "bevy_platform_support/critical-section", ] -## `portable-atomic` provides additional platform support for atomic types and -## operations, even on targets without native support. -portable-atomic = [ - "bevy_ecs/portable-atomic", - "bevy_utils/portable-atomic", - "bevy_app?/portable-atomic", - "bevy_reflect?/portable-atomic", - "bevy_platform_support/portable-atomic", -] - [dependencies] # bevy bevy_ecs = { path = "../bevy_ecs", version = "0.16.0-dev", default-features = false } diff --git a/crates/bevy_tasks/Cargo.toml b/crates/bevy_tasks/Cargo.toml index 089aa917c1e27..3f9c9f2a3a995 100644 --- a/crates/bevy_tasks/Cargo.toml +++ b/crates/bevy_tasks/Cargo.toml @@ -23,11 +23,6 @@ critical-section = [ "bevy_platform_support/critical-section", "edge-executor?/critical-section", ] -portable-atomic = [ - "bevy_platform_support/portable-atomic", - "edge-executor?/portable-atomic", - "async-task/portable-atomic", -] [dependencies] bevy_platform_support = { path = "../bevy_platform_support", version = "0.16.0-dev", default-features = false, features = [ @@ -54,6 +49,14 @@ wasm-bindgen-futures = "0.4" pin-project = "1" futures-channel = "0.3" +[target.'cfg(not(all(target_has_atomic = "8", target_has_atomic = "16", target_has_atomic = "32", target_has_atomic = "64", target_has_atomic = "ptr")))'.dependencies] +async-task = { version = "4.4.0", default-features = false, features = [ + "portable-atomic", +] } +edge-executor = { version = "0.4.1", default-features = false, optional = true, features = [ + "portable-atomic", +] } + [dev-dependencies] web-time = { version = "1.1" } diff --git a/crates/bevy_tasks/src/task_pool.rs b/crates/bevy_tasks/src/task_pool.rs index b9c0e9f98261a..661e5bce2b419 100644 --- a/crates/bevy_tasks/src/task_pool.rs +++ b/crates/bevy_tasks/src/task_pool.rs @@ -74,16 +74,20 @@ impl TaskPoolBuilder { /// This is called on the thread itself and has access to all thread-local storage. /// This will block running async tasks on the thread until the callback completes. pub fn on_thread_spawn(mut self, f: impl Fn() + Send + Sync + 'static) -> Self { - #[cfg(feature = "portable-atomic")] - let arc = { - let boxed = Box::new(f); - let boxed: Box = boxed; - Arc::from(boxed) - }; - - #[cfg(not(feature = "portable-atomic"))] let arc = Arc::new(f); + #[cfg(not(target_has_atomic = "ptr"))] + #[expect( + unsafe_code, + reason = "unsized coercion is an unstable feature for non-std types" + )] + // SAFETY: + // - Coercion from `impl Fn` to `dyn Fn` is valid + // - `Arc::from_raw` receives a valid pointer from a previous call to `Arc::into_raw` + let arc = unsafe { + Arc::from_raw(Arc::into_raw(arc) as *const (dyn Fn() + Send + Sync + 'static)) + }; + self.on_thread_spawn = Some(arc); self } @@ -93,16 +97,20 @@ impl TaskPoolBuilder { /// This is called on the thread itself and has access to all thread-local storage. /// This will block thread termination until the callback completes. pub fn on_thread_destroy(mut self, f: impl Fn() + Send + Sync + 'static) -> Self { - #[cfg(feature = "portable-atomic")] - let arc = { - let boxed = Box::new(f); - let boxed: Box = boxed; - Arc::from(boxed) - }; - - #[cfg(not(feature = "portable-atomic"))] let arc = Arc::new(f); + #[cfg(not(target_has_atomic = "ptr"))] + #[expect( + unsafe_code, + reason = "unsized coercion is an unstable feature for non-std types" + )] + // SAFETY: + // - Coercion from `impl Fn` to `dyn Fn` is valid + // - `Arc::from_raw` receives a valid pointer from a previous call to `Arc::into_raw` + let arc = unsafe { + Arc::from_raw(Arc::into_raw(arc) as *const (dyn Fn() + Send + Sync + 'static)) + }; + self.on_thread_destroy = Some(arc); self } diff --git a/crates/bevy_time/Cargo.toml b/crates/bevy_time/Cargo.toml index 97fcda1b0cd6b..84db6a457c1b7 100644 --- a/crates/bevy_time/Cargo.toml +++ b/crates/bevy_time/Cargo.toml @@ -50,15 +50,6 @@ critical-section = [ "bevy_app/critical-section", ] -## `portable-atomic` provides additional platform support for atomic types and -## operations, even on targets without native support. -portable-atomic = [ - "bevy_ecs/portable-atomic", - "bevy_platform_support/portable-atomic", - "bevy_reflect?/portable-atomic", - "bevy_app/portable-atomic", -] - [dependencies] # bevy bevy_app = { path = "../bevy_app", version = "0.16.0-dev", default-features = false } diff --git a/crates/bevy_utils/Cargo.toml b/crates/bevy_utils/Cargo.toml index ce50c4c45f34a..2e8e9dc31494f 100644 --- a/crates/bevy_utils/Cargo.toml +++ b/crates/bevy_utils/Cargo.toml @@ -30,10 +30,6 @@ alloc = ["bevy_platform_support/alloc"] ## on all platforms, including `no_std`. critical-section = ["bevy_platform_support/critical-section"] -## `portable-atomic` provides additional platform support for atomic types and -## operations, even on targets without native support. -portable-atomic = ["bevy_platform_support/portable-atomic"] - [dependencies] bevy_platform_support = { path = "../bevy_platform_support", version = "0.16.0-dev", default-features = false }