Skip to content

Reorganise Platform Support Dependencies #16877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bushrat011899 opened this issue Dec 18, 2024 · 2 comments
Closed

Reorganise Platform Support Dependencies #16877

bushrat011899 opened this issue Dec 18, 2024 · 2 comments
Assignees
Labels
A-Cross-Cutting Impacts the entire engine A-Utils Utility functions and types C-Code-Quality A section of code that is hard to understand or change C-Dependencies A change to the crates that Bevy depends on D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Contentious There are nontrivial implications that should be thought through

Comments

@bushrat011899
Copy link
Contributor

bushrat011899 commented Dec 18, 2024

I'm not a fan of how we bring this Instant in. I think it'd be worth considering some kind of bevy_platform_support crate which moves things like spin, portable-atomic, critical-section, wasm-time, etc. into a single spot. It'd be more focussed than bevy_utils, and possibly clean up some of the cfg(...) noise we have across Bevy.

Originally posted by @bushrat011899 in #16874 (comment)

@bushrat011899 bushrat011899 added C-Dependencies A change to the crates that Bevy depends on C-Code-Quality A section of code that is hard to understand or change S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! A-Cross-Cutting Impacts the entire engine D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Dec 18, 2024
@alice-i-cecile alice-i-cecile added X-Contentious There are nontrivial implications that should be thought through A-Utils Utility functions and types labels Dec 18, 2024
@alice-i-cecile
Copy link
Member

Part of #11478.

@bushrat011899 bushrat011899 self-assigned this Dec 19, 2024
github-merge-queue bot pushed a commit that referenced this issue Jan 5, 2025
# Objective

- Contributes to #11478
- Contributes to #16877

## Solution

- Removed everything except `Instant` from `bevy_utils::time`

## Testing

- CI

---

## Migration Guide

If you relied on any of the following from `bevy_utils::time`:

- `Duration`
- `TryFromFloatSecsError`

Import these directly from `core::time` regardless of platform target
(WASM, mobile, etc.)

If you relied on any of the following from `bevy_utils::time`:

- `SystemTime`
- `SystemTimeError`

Instead import these directly from either `std::time` or `web_time` as
appropriate for your target platform.

## Notes

`Duration` and `TryFromFloatSecsError` are both re-exports from
`core::time` regardless of whether they are used from `web_time` or
`std::time`, so there is no value gained from re-exporting them from
`bevy_utils::time` as well. As for `SystemTime` and `SystemTimeError`,
no Bevy internal crates or examples rely on these types. Since Bevy
doesn't have a `Time<Wall>` resource for interacting with wall-time (and
likely shouldn't need one), I think removing these from `bevy_utils`
entirely and waiting for a use-case to justify inclusion is a reasonable
path forward.
github-merge-queue bot pushed a commit that referenced this issue Jan 20, 2025
# Objective

- Contributes to #16877

## Solution

- Initial creation of `bevy_platform_support` crate.
- Moved `bevy_utils::Instant` into new `bevy_platform_support` crate.
- Moved `portable-atomic`, `portable-atomic-util`, and
`critical-section` into new `bevy_platform_support` crate.

## Testing

- CI

---

## Showcase

Instead of needing code like this to import an `Arc`:

```rust
#[cfg(feature = "portable-atomic")]
use portable_atomic_util::Arc;

#[cfg(not(feature = "portable-atomic"))]
use alloc::sync::Arc;
```

We can now use:

```rust
use bevy_platform_support::sync::Arc;
```

This applies to many other types, but the goal is overall the same:
allowing crates to use `std`-like types without the boilerplate of
conditional compilation and platform-dependencies.

## Migration Guide

- Replace imports of `bevy_utils::Instant` with
`bevy_platform_support::time::Instant`
- Replace imports of `bevy::utils::Instant` with
`bevy::platform_support::time::Instant`

## Notes

- `bevy_platform_support` hasn't been reserved on `crates.io`
- ~~`bevy_platform_support` is not re-exported from `bevy` at this time.
It may be worthwhile exporting this crate, but I am unsure of a
reasonable name to export it under (`platform_support` may be a bit
wordy for user-facing).~~
- I've included an implementation of `Instant` which is suitable for
`no_std` platforms that are not Wasm for the sake of eliminating feature
gates around its use. It may be a controversial inclusion, so I'm happy
to remove it if required.
- There are many other items (`spin`, `bevy_utils::Sync(Unsafe)Cell`,
etc.) which should be added to this crate. I have kept the initial scope
small to demonstrate utility without making this too unwieldy.

---------

Co-authored-by: TimJentzsch <[email protected]>
Co-authored-by: Chris Russell <[email protected]>
Co-authored-by: François Mockers <[email protected]>
github-merge-queue bot pushed a commit that referenced this issue Jan 23, 2025
# Objective

- Contributes to #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]>
github-merge-queue bot pushed a commit that referenced this issue Jan 23, 2025
# Objective

- Contributes to #16877

## Solution

- Moved `hashbrown`, `foldhash`, and related types out of `bevy_utils`
and into `bevy_platform_support`
- Refactored the above to match the layout of these types in `std`.
- Updated crates as required.

## Testing

- CI

---

## Migration Guide

- The following items were moved out of `bevy_utils` and into
`bevy_platform_support::hash`:
  - `FixedState`
  - `DefaultHasher`
  - `RandomState`
  - `FixedHasher`
  - `Hashed`
  - `PassHash`
  - `PassHasher`
  - `NoOpHash`
- The following items were moved out of `bevy_utils` and into
`bevy_platform_support::collections`:
  - `HashMap`
  - `HashSet`
- `bevy_utils::hashbrown` has been removed. Instead, import from
`bevy_platform_support::collections` _or_ take a dependency on
`hashbrown` directly.
- `bevy_utils::Entry` has been removed. Instead, import from
`bevy_platform_support::collections::hash_map` or
`bevy_platform_support::collections::hash_set` as appropriate.
- All of the above equally apply to `bevy::utils` and
`bevy::platform_support`.

## Notes

- I left `PreHashMap`, `PreHashMapExt`, and `TypeIdMap` in `bevy_utils`
as they might be candidates for micro-crating. They can always be moved
into `bevy_platform_support` at a later date if desired.
@bushrat011899
Copy link
Contributor Author

Closing as completed by the various contributing PRs above!

mrchantey pushed a commit to mrchantey/bevy that referenced this issue Feb 4, 2025
…e#17158)

# Objective

- Contributes to bevyengine#11478
- Contributes to bevyengine#16877

## Solution

- Removed everything except `Instant` from `bevy_utils::time`

## Testing

- CI

---

## Migration Guide

If you relied on any of the following from `bevy_utils::time`:

- `Duration`
- `TryFromFloatSecsError`

Import these directly from `core::time` regardless of platform target
(WASM, mobile, etc.)

If you relied on any of the following from `bevy_utils::time`:

- `SystemTime`
- `SystemTimeError`

Instead import these directly from either `std::time` or `web_time` as
appropriate for your target platform.

## Notes

`Duration` and `TryFromFloatSecsError` are both re-exports from
`core::time` regardless of whether they are used from `web_time` or
`std::time`, so there is no value gained from re-exporting them from
`bevy_utils::time` as well. As for `SystemTime` and `SystemTimeError`,
no Bevy internal crates or examples rely on these types. Since Bevy
doesn't have a `Time<Wall>` resource for interacting with wall-time (and
likely shouldn't need one), I think removing these from `bevy_utils`
entirely and waiting for a use-case to justify inclusion is a reasonable
path forward.
mrchantey pushed a commit to mrchantey/bevy that referenced this issue Feb 4, 2025
# Objective

- Contributes to bevyengine#16877

## Solution

- Initial creation of `bevy_platform_support` crate.
- Moved `bevy_utils::Instant` into new `bevy_platform_support` crate.
- Moved `portable-atomic`, `portable-atomic-util`, and
`critical-section` into new `bevy_platform_support` crate.

## Testing

- CI

---

## Showcase

Instead of needing code like this to import an `Arc`:

```rust
#[cfg(feature = "portable-atomic")]
use portable_atomic_util::Arc;

#[cfg(not(feature = "portable-atomic"))]
use alloc::sync::Arc;
```

We can now use:

```rust
use bevy_platform_support::sync::Arc;
```

This applies to many other types, but the goal is overall the same:
allowing crates to use `std`-like types without the boilerplate of
conditional compilation and platform-dependencies.

## Migration Guide

- Replace imports of `bevy_utils::Instant` with
`bevy_platform_support::time::Instant`
- Replace imports of `bevy::utils::Instant` with
`bevy::platform_support::time::Instant`

## Notes

- `bevy_platform_support` hasn't been reserved on `crates.io`
- ~~`bevy_platform_support` is not re-exported from `bevy` at this time.
It may be worthwhile exporting this crate, but I am unsure of a
reasonable name to export it under (`platform_support` may be a bit
wordy for user-facing).~~
- I've included an implementation of `Instant` which is suitable for
`no_std` platforms that are not Wasm for the sake of eliminating feature
gates around its use. It may be a controversial inclusion, so I'm happy
to remove it if required.
- There are many other items (`spin`, `bevy_utils::Sync(Unsafe)Cell`,
etc.) which should be added to this crate. I have kept the initial scope
small to demonstrate utility without making this too unwieldy.

---------

Co-authored-by: TimJentzsch <[email protected]>
Co-authored-by: Chris Russell <[email protected]>
Co-authored-by: François Mockers <[email protected]>
mrchantey pushed a commit to mrchantey/bevy that referenced this issue 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]>
mrchantey pushed a commit to mrchantey/bevy that referenced this issue Feb 4, 2025
# Objective

- Contributes to bevyengine#16877

## Solution

- Moved `hashbrown`, `foldhash`, and related types out of `bevy_utils`
and into `bevy_platform_support`
- Refactored the above to match the layout of these types in `std`.
- Updated crates as required.

## Testing

- CI

---

## Migration Guide

- The following items were moved out of `bevy_utils` and into
`bevy_platform_support::hash`:
  - `FixedState`
  - `DefaultHasher`
  - `RandomState`
  - `FixedHasher`
  - `Hashed`
  - `PassHash`
  - `PassHasher`
  - `NoOpHash`
- The following items were moved out of `bevy_utils` and into
`bevy_platform_support::collections`:
  - `HashMap`
  - `HashSet`
- `bevy_utils::hashbrown` has been removed. Instead, import from
`bevy_platform_support::collections` _or_ take a dependency on
`hashbrown` directly.
- `bevy_utils::Entry` has been removed. Instead, import from
`bevy_platform_support::collections::hash_map` or
`bevy_platform_support::collections::hash_set` as appropriate.
- All of the above equally apply to `bevy::utils` and
`bevy::platform_support`.

## Notes

- I left `PreHashMap`, `PreHashMapExt`, and `TypeIdMap` in `bevy_utils`
as they might be candidates for micro-crating. They can always be moved
into `bevy_platform_support` at a later date if desired.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Cross-Cutting Impacts the entire engine A-Utils Utility functions and types C-Code-Quality A section of code that is hard to understand or change C-Dependencies A change to the crates that Bevy depends on D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

No branches or pull requests

2 participants