Move spin to bevy_platform_support out of other crates
#17470
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bushrat011899
added
C-Code-Quality
|
Marking as |
BenjaminBrienen
approved these changes
Jan 21, 2025
Comment on lines
-146
to
-163
| #[cfg(feature = "std")] | ||
| let set = self.0.read().unwrap_or_else(PoisonError::into_inner); | ||
|
|
||
| #[cfg(not(feature = "std"))] | ||
| let set = self.0.read(); | ||
|
|
||
| if let Some(value) = set.get(value) { | ||
| return Interned(*value); | ||
| } | ||
| } | ||
|
|
||
| { | ||
| #[cfg(feature = "std")] | ||
| let mut set = self.0.write().unwrap_or_else(PoisonError::into_inner); | ||
|
|
||
| #[cfg(not(feature = "std"))] | ||
| let mut set = self.0.write(); | ||
|
|
There was a problem hiding this comment.
I like how this cleans up
There was a problem hiding this comment.
That's exactly my goal here. It's technically more lines of code total, but it's far easier to reason about.
crates/bevy_tasks/src/usages.rs
Outdated
BenjaminBrienen
approved these changes
Jan 21, 2025
BD103
approved these changes
Jan 21, 2025
Most importantly, highlighting which items _would_ be in `std` which _aren't_ available. Co-Authored-By: BD103 <[email protected]> Co-Authored-By: Benjamin Brienen <[email protected]>
bushrat011899
added
S-Ready-For-Final-Review
mrchantey
pushed a commit
to mrchantey/bevy
that referenced
this pull request
Feb 4, 2025
…e#17470) # Objective - Contributes to bevyengine#16877 ## Solution - Expanded `bevy_platform_support::sync` module to provide API-compatible replacements for `std` items such as `RwLock`, `Mutex`, and `OnceLock`. - Removed `spin` from all crates except `bevy_platform_support`. ## Testing - CI --- ## Notes - The sync primitives, while verbose, entirely rely on `spin` for their implementation requiring no `unsafe` and not changing the status-quo on _how_ locks actually work within Bevy. This is just a refactoring to consolidate the "hacks" and workarounds required to get a consistent experience when either using `std::sync` or `spin`. - I have opted to rely on `std::sync` for `std` compatible locks, maintaining the status quo. However, now that we have these locks factored out into the own module, it would be trivial to investigate alternate locking backends, such as `parking_lot`. - API for these locking types is entirely based on `std`. I have implemented methods and types which aren't currently in use within Bevy (e.g., `LazyLock` and `Once`) for the sake of completeness. As the standard library is highly stable, I don't expect the Bevy and `std` implementations to drift apart much if at all. --------- Co-authored-by: BD103 <[email protected]> Co-authored-by: Benjamin Brienen <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
Solution
bevy_platform_support::syncmodule to provide API-compatible replacements forstditems such asRwLock,Mutex, andOnceLock.spinfrom all crates exceptbevy_platform_support.Testing
Notes
spinfor their implementation requiring nounsafeand not changing the status-quo on how locks actually work within Bevy. This is just a refactoring to consolidate the "hacks" and workarounds required to get a consistent experience when either usingstd::syncorspin.std::syncforstdcompatible locks, maintaining the status quo. However, now that we have these locks factored out into the own module, it would be trivial to investigate alternate locking backends, such asparking_lot.std. I have implemented methods and types which aren't currently in use within Bevy (e.g.,LazyLockandOnce) for the sake of completeness. As the standard library is highly stable, I don't expect the Bevy andstdimplementations to drift apart much if at all.