MVP `no_std` for `wgpu-core`
Connections
- Contributes to #6826
Description
Adds no_std support to wgpu-core through 3 changes:
- Use
num-traitswithintimestamp_normalizationto account for the floating point methods only available withstd. - Make
parking_lotoptional by falling back to eitherstd::sync::{Mutex, RwLock},spin::{Mutex, RwLock}, orcore::cell::RefCellbased on selecting eitherparking_lot,std,spin, or no locking implementation. a. Note thatspinwas already in the lockfile viacrossbeam-dequeso this feature is "free" b. Note thatRefCellis!Sync, so users should selectspininno_stdenvironments. The use ofRefCellas a fallback just allows compilation to succeed without any features enabled. - Make
once_celloptional by falling back tocore::cell:OnceCell. a. Note thatonce_cellisno_stdcompatible, but it requires activating thecritical-sectionfeature to access its implementation ofOnceCell. Adding such a feature would therefore addcritical-sectionto the lockfile, requiring thorough review due to its use ofunsafeandextern "Rust". I believe it is a safe inclusion, but it can be avoided here so I'd rather save that quality of life feature for a follow-up. Users are still able to useonce_cellwithwgpu-coreonno_std, they just have to enableonce_cell/critical-sectionthemselves. b. Note thatcore::cell::OnceCellis!Sync, so users should prefer to enablecritical-sectionthemselves.
To preserve existing behavior, the newly added parking_lot and once_cell features are enabled by default.
Testing
Added wgpu-core to the wasm32v1-none CI test.
Squash or Rebase?
Squash
Checklist
- [X] Run
cargo fmt. - [X] Run
taplo format. - [x] Run
cargo clippy --tests. If applicable, add:- [x]
--target wasm32-unknown-unknown
- [x]
- [x] Run
cargo xtask testto run tests. - [x] If this contains user-facing changes, add a
CHANGELOG.mdentry.
It irks me to add yet another feature just because of that, also given that
once_cellis such a small and common crate. I think if we could we'd want to imply not enablingstdto enableonce_cell/critical-section. Since this isn't possible I'd instead argue it's better to just keep always depending ononce_celland point out to users that they have to useonce_cell/critical-sectionif they wantSync. That's already the case with the PR as-is, only that a user also has to enable theonce_cellfeature. This would save us feature permutations, an extra code path and complexity for users. What do you think?
Agreed! The ideal solution (that we can implement without changes to Cargo or rustc of course) would just be to make once_cell a required dependency and add a critical-section feature to enable Sync support. This can be done, but doing so adds critical-section to the Cargo.lock file, which would necessitate a review of that crate and its usage of unsafe (which it does use). I'm happy to refactor this PR to do this, but I'm unsure if adding critical-section to the lock file is a non-starter.
I've split out the Mutex and RwLock changes into #7830, as they are a good fit for wgpu-types and would help with Naga and wgpu-hal too.
Sorry for the long lead times on these. It's been quite busy at work and I'm now out for a couple of days. Will likely get to it some point next week 🤞
No stress! Thanks for your help 🙂
finally getting coming back to this after a break! First of all, thanks for all the added docs, looks great now. I'd like to discuss ways to reduce amount of features a bit more though, after all this quite significantly increases the number of permutations and thus technically the number of versions that ought to be tested.
The ideal solution (that we can implement without changes to Cargo or rustc of course) would just be to make once_cell a required dependency and add a critical-section feature to enable Sync support.
Not entirely sure I'm following it all the way: right now the PR exposes once_cell as a separate feature and expects anyone that wants to use no_std to do once_cell/critical-section if they want resource pool to be Sync. In other words there's a way to get to Sync but it requires --features once_cell/critical-section. If we were to drop once_cell feature and always depend on once_cell would this change then, or does cargo no longer allow --features once_cell/critical-section and requires you instead to add this to your application's Cargo.toml?
But either way we still end up with imho way to many hard to use feature knobs. I'm wondering if we instead should adjust a little bit:
- have
parking_lotbe directly implied bystdwithout aparking_lotfeature-
parking_lotcan't be used in a no_std environment anyways and it's what we always used forstdso far
-
- two features to configure no_std - if cargo would allow it I'd definitely force-auto-enable those whenever
stdis disabled to cut down on things, but alas we can't:- add
spinfeature as done in this pr, have it yield to std like it yields in hits pr right now toparking_lot - add
critical-sectionfeature as you've described, replacingonce_cellfeature
- add
@ErichDonGubler does Mozilla care about dependencies that show up in the lock file but aren't ever enabled in Firefox? Should be fine, after all we also have testing dependencies in the same situation, no?
@ErichDonGubler does Mozilla care about dependencies that show up in the lock file but aren't ever enabled in Firefox? Should be fine, after all we also have testing dependencies in the same situation, no?
So long as they don't show up in mozilla-central's Cargo.lock file, then 🤷🏻♂️ it's not something we have to vet, and therefore not something we might have to reject.
great! :)
@bushrat011899 sounds like we could above mentioned suggestion then, what do you think?
- have
parking_lotbe directly implied bystdwithout aparking_lotfeature
parking_lotcan't be used in a no_std environment anyways and it's what we always used forstdso far- two features to configure no_std - if cargo would allow it I'd definitely force-auto-enable those whenever
stdis disabled to cut down on things, but alas we can't:
- add
spinfeature as done in this pr, have it yield to std like it yields in hits pr right now toparking_lot- add
critical-sectionfeature as you've described, replacingonce_cellfeature
@bushrat011899 poke!
Hey! I know personal life got busy for you, take your time, but I'm going to demote these two PRs to drafts, and you can re-promote them when they're ready for review again!
Closing this as stale, if you want to pick up this work again, please re-open a PR with an updated branch.