From c9628ca7013ddc25e6b405c03fb8f75020c26af4 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Fri, 15 Dec 2023 14:10:41 +0000 Subject: [PATCH 01/12] Allow usage on the main web thread with wasm-sync One of the most common complaints I've been receiving in [wasm-bindgen-rayon](https://github.com/RReverser/wasm-bindgen-rayon) that prevents people from using Rayon on the Web is the complexity of manually splitting up the code that uses Rayon into a Web Worker from code that drives the UI. It requires custom message passing for proxying between two threads (Workers), which, admittedly, feels particularly silly when using a tool that is meant to simplify working with threads for you. This all stems from a [Wasm limitation](https://github.com/WebAssembly/threads/issues/177) that disallows `atomic.wait` on the main browser thread. In theory, it's a reasonable limitation, since blocking main thread on the web is more problematic than on other platforms as it blocks web app's UI from being responsive altogether, and because there is no limit on how long atomic wait can block. In practice, however, it causes enough issues for users that various toolchains - even Emscripten - work around this issue by spin-locking when on the main thread. Rust / wasm-bindgen decided not to adopt the same workaround, following general Wasm limitation, which is also a fair stance for general implementation of `Mutex` and other blocking primitives, but I believe Rayon usecase is quite different. Code using parallel iterators is almost always guaranteed to run for less or, worst-case, ~same time as code using regular iterators, so it doesn't make sense to "punish" Rayon users and prevent them from being able to use parallel iterators on the main thread when it will lead to a _more_ responsive UI than using regular iterators. This PR adds a `cfg`-conditional dependency on [wasm_sync](https://docs.rs/wasm_sync/latest/wasm_sync/) that automatically switches to allowed spin-based `Mutex` and `Condvar` when it detects it's running on the main thread, and to regular `std::sync` based implementation otherwise, thus avoiding the `atomics.wait` error. This dependency will only be added when building for `wasm32-unknown-unknown` - that is, not affecting WASI and Emscripten users - and only when building with `-C target-feature=+atomics`, so not affecting users who rely on Rayon's single-threaded fallback mode either. I hope this kind of very limited override will be acceptable as it makes it much easier to use Rayon on the web. When this is merged, I'll be able to leverage it in wasm-bindgen-rayon and [significantly simplify](https://github.com/RReverser/wasm-bindgen-rayon/compare/main...RReverser:wasm-bindgen-rayon:wasm-sync?expand=1) demos, tests and docs by avoiding that extra Worker machinery (e.g. see `demo/wasm-worker.js` and `demo/index.js` merged into single simple JS file in the linked diff). --- rayon-core/Cargo.toml | 3 +++ rayon-core/src/latch.rs | 3 ++- rayon-core/src/lib.rs | 14 ++++++++++++++ rayon-core/src/registry.rs | 3 ++- rayon-core/src/sleep/mod.rs | 2 +- src/iter/par_bridge.rs | 2 +- src/result.rs | 2 +- 7 files changed, 24 insertions(+), 5 deletions(-) diff --git a/rayon-core/Cargo.toml b/rayon-core/Cargo.toml index 2cd5372bb..f8793352a 100644 --- a/rayon-core/Cargo.toml +++ b/rayon-core/Cargo.toml @@ -20,6 +20,9 @@ categories = ["concurrency"] crossbeam-deque = "0.8.1" crossbeam-utils = "0.8.0" +[target.'cfg(all(target_arch = "wasm32", target_os = "unknown", target_feature = "atomics"))'.dependencies] +wasm_sync = "0.1.0" + [dev-dependencies] rand = "0.8" rand_xorshift = "0.3" diff --git a/rayon-core/src/latch.rs b/rayon-core/src/latch.rs index b0cbbd833..6c2e4fe97 100644 --- a/rayon-core/src/latch.rs +++ b/rayon-core/src/latch.rs @@ -1,10 +1,11 @@ use std::marker::PhantomData; use std::ops::Deref; use std::sync::atomic::{AtomicUsize, Ordering}; -use std::sync::{Arc, Condvar, Mutex}; +use std::sync::Arc; use std::usize; use crate::registry::{Registry, WorkerThread}; +use crate::sync::{Condvar, Mutex}; /// We define various kinds of latches, which are all a primitive signaling /// mechanism. A latch starts as false. Eventually someone calls `set()` and diff --git a/rayon-core/src/lib.rs b/rayon-core/src/lib.rs index 7001c8c1d..9a9eb3fa4 100644 --- a/rayon-core/src/lib.rs +++ b/rayon-core/src/lib.rs @@ -103,6 +103,20 @@ pub use self::thread_pool::current_thread_index; pub use self::thread_pool::ThreadPool; pub use self::thread_pool::{yield_local, yield_now, Yield}; +#[cfg(not(all( + target_arch = "wasm32", + target_os = "unknown", + target_feature = "atomics" +)))] +pub use std::sync; + +#[cfg(all( + target_arch = "wasm32", + target_os = "unknown", + target_feature = "atomics" +))] +pub use wasm_sync as sync; + use self::registry::{CustomSpawn, DefaultSpawn, ThreadSpawn}; /// Returns the maximum number of threads that Rayon supports in a single thread-pool. diff --git a/rayon-core/src/registry.rs b/rayon-core/src/registry.rs index e4f2ac7cd..46cd22b31 100644 --- a/rayon-core/src/registry.rs +++ b/rayon-core/src/registry.rs @@ -1,6 +1,7 @@ use crate::job::{JobFifo, JobRef, StackJob}; use crate::latch::{AsCoreLatch, CoreLatch, Latch, LatchRef, LockLatch, OnceLatch, SpinLatch}; use crate::sleep::Sleep; +use crate::sync::Mutex; use crate::unwind; use crate::{ ErrorKind, ExitHandler, PanicHandler, StartHandler, ThreadPoolBuildError, ThreadPoolBuilder, @@ -15,7 +16,7 @@ use std::io; use std::mem; use std::ptr; use std::sync::atomic::{AtomicUsize, Ordering}; -use std::sync::{Arc, Mutex, Once}; +use std::sync::{Arc, Once}; use std::thread; use std::usize; diff --git a/rayon-core/src/sleep/mod.rs b/rayon-core/src/sleep/mod.rs index 03d1077f7..fa1f7beed 100644 --- a/rayon-core/src/sleep/mod.rs +++ b/rayon-core/src/sleep/mod.rs @@ -2,9 +2,9 @@ //! for an overview. use crate::latch::CoreLatch; +use crate::sync::{Condvar, Mutex}; use crossbeam_utils::CachePadded; use std::sync::atomic::Ordering; -use std::sync::{Condvar, Mutex}; use std::thread; use std::usize; diff --git a/src/iter/par_bridge.rs b/src/iter/par_bridge.rs index eb058d3e6..3e7d65d79 100644 --- a/src/iter/par_bridge.rs +++ b/src/iter/par_bridge.rs @@ -1,5 +1,5 @@ +use rayon_core::sync::Mutex; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; -use std::sync::Mutex; use crate::iter::plumbing::{bridge_unindexed, Folder, UnindexedConsumer, UnindexedProducer}; use crate::iter::ParallelIterator; diff --git a/src/result.rs b/src/result.rs index 43685ca43..d6f84e25f 100644 --- a/src/result.rs +++ b/src/result.rs @@ -7,7 +7,7 @@ use crate::iter::plumbing::*; use crate::iter::*; -use std::sync::Mutex; +use rayon_core::sync::Mutex; use crate::option; From 23fb942147a3a2a7731fb78b855845a4bffa4570 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Tue, 2 Jan 2024 15:44:27 +0000 Subject: [PATCH 02/12] Use regular Mutex where only try_lock is used --- src/result.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/result.rs b/src/result.rs index d6f84e25f..43685ca43 100644 --- a/src/result.rs +++ b/src/result.rs @@ -7,7 +7,7 @@ use crate::iter::plumbing::*; use crate::iter::*; -use rayon_core::sync::Mutex; +use std::sync::Mutex; use crate::option; From 1d880a60e3ea6fb736f7b67b4dc6fdc030c0115b Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Wed, 10 Jan 2024 22:21:25 +0000 Subject: [PATCH 03/12] Make wasm_sync private dependency --- Cargo.toml | 3 +++ rayon-core/src/lib.rs | 4 ++-- src/iter/par_bridge.rs | 15 ++++++++++++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index bd019dbc9..cc7a69e30 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,6 +21,9 @@ exclude = ["ci"] [dependencies] rayon-core = { version = "1.12.0", path = "rayon-core" } +[target.'cfg(all(target_arch = "wasm32", target_os = "unknown", target_feature = "atomics"))'.dependencies] +wasm_sync = "0.1.0" + # This is a public dependency! [dependencies.either] version = "1.0" diff --git a/rayon-core/src/lib.rs b/rayon-core/src/lib.rs index 9a9eb3fa4..b9dfdf019 100644 --- a/rayon-core/src/lib.rs +++ b/rayon-core/src/lib.rs @@ -108,14 +108,14 @@ pub use self::thread_pool::{yield_local, yield_now, Yield}; target_os = "unknown", target_feature = "atomics" )))] -pub use std::sync; +use std::sync; #[cfg(all( target_arch = "wasm32", target_os = "unknown", target_feature = "atomics" ))] -pub use wasm_sync as sync; +use wasm_sync as sync; use self::registry::{CustomSpawn, DefaultSpawn, ThreadSpawn}; diff --git a/src/iter/par_bridge.rs b/src/iter/par_bridge.rs index 3e7d65d79..ac801af92 100644 --- a/src/iter/par_bridge.rs +++ b/src/iter/par_bridge.rs @@ -1,4 +1,17 @@ -use rayon_core::sync::Mutex; +#[cfg(not(all( + target_arch = "wasm32", + target_os = "unknown", + target_feature = "atomics" +)))] +use std::sync::Mutex; + +#[cfg(all( + target_arch = "wasm32", + target_os = "unknown", + target_feature = "atomics" +))] +use wasm_sync::Mutex; + use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use crate::iter::plumbing::{bridge_unindexed, Folder, UnindexedConsumer, UnindexedProducer}; From ebc4539ecaebd90a70a70570e29f73b374c83d8d Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Wed, 10 Jan 2024 22:23:04 +0000 Subject: [PATCH 04/12] Update compat-Cargo.lock --- ci/compat-Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/compat-Cargo.lock b/ci/compat-Cargo.lock index ca7d0737b..fd9538b07 100644 --- a/ci/compat-Cargo.lock +++ b/ci/compat-Cargo.lock @@ -1065,6 +1065,7 @@ dependencies = [ "rand", "rand_xorshift", "rayon-core", + "wasm_sync", ] [[package]] From bc009a38cad598e706a4167eb65fe7bd42ca7ae8 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Wed, 10 Jan 2024 22:25:02 +0000 Subject: [PATCH 05/12] Update compat-Cargo.lock --- ci/compat-Cargo.lock | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ci/compat-Cargo.lock b/ci/compat-Cargo.lock index fd9538b07..371bdb4bd 100644 --- a/ci/compat-Cargo.lock +++ b/ci/compat-Cargo.lock @@ -1078,6 +1078,7 @@ dependencies = [ "rand", "rand_xorshift", "scoped-tls", + "wasm_sync", ] [[package]] @@ -1477,6 +1478,15 @@ version = "0.2.89" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7ab9b36309365056cd639da3134bf87fa8f3d86008abf99e612384a6eecd459f" +[[package]] +name = "wasm_sync" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4f22b3c2526c5834350ca8de37292cbd2cb7724e6c812930cfb8c558340cf76f" +dependencies = [ + "web-sys", +] + [[package]] name = "wayland-backend" version = "0.3.2" From 085f284f67510aceb486345b15cac4615be9218d Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Fri, 12 Jan 2024 15:30:31 +0000 Subject: [PATCH 06/12] Split up CI --- .github/workflows/ci.yaml | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index a25c1367e..d15f39d08 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -65,7 +65,29 @@ jobs: # unsupported threading, but we don't have an environment to execute in. # wasm32-wasi can test the fallback by running in wasmtime. wasm: - name: WebAssembly + name: WebAssembly (standalone) + runs-on: ubuntu-latest + strategy: + matrix: + include: + - toolchain: stable + targets: wasm32-unknown-unknown + - toolchain: nightly + targets: wasm32-unknown-unknown + rustflags: -C target-feature=+atomics,+bulk-memory,+mutable-globals + env: + RUSTFLAGS: ${{ matrix.rustflags }} + steps: + - uses: actions/checkout@v3 + - uses: dtolnay/rust-toolchain@master + with: + toolchain: ${{ matrix.toolchain }} + targets: wasm32-unknown-unknown + - run: cargo build --verbose --target wasm32-unknown-unknown + + # wasm32-wasi can test the fallback by running in wasmtime. + wasi: + name: WebAssembly (WASI) runs-on: ubuntu-latest env: CARGO_TARGET_WASM32_WASI_RUNNER: /home/runner/.wasmtime/bin/wasmtime @@ -73,10 +95,9 @@ jobs: - uses: actions/checkout@v3 - uses: dtolnay/rust-toolchain@stable with: - targets: wasm32-unknown-unknown,wasm32-wasi - - run: cargo check --verbose --target wasm32-unknown-unknown - - run: cargo check --verbose --target wasm32-wasi + targets: wasm32-wasi - run: curl https://wasmtime.dev/install.sh -sSf | bash + - run: cargo build --verbose --target wasm32-wasi - run: cargo test --verbose --target wasm32-wasi --package rayon - run: cargo test --verbose --target wasm32-wasi --package rayon-core From 2897edc783fc08a92f8dec07ae1594d4c30eeddb Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Fri, 12 Jan 2024 15:39:23 +0000 Subject: [PATCH 07/12] Remove unused matrix variable --- .github/workflows/ci.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index d15f39d08..ddf985109 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -71,9 +71,7 @@ jobs: matrix: include: - toolchain: stable - targets: wasm32-unknown-unknown - toolchain: nightly - targets: wasm32-unknown-unknown rustflags: -C target-feature=+atomics,+bulk-memory,+mutable-globals env: RUSTFLAGS: ${{ matrix.rustflags }} From 9ea9317b2e62ea8a771515752528bde4b53cb337 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Fri, 12 Jan 2024 19:20:09 +0000 Subject: [PATCH 08/12] Update .github/workflows/ci.yaml Co-authored-by: Josh Stone --- .github/workflows/ci.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index ddf985109..358bc6f4f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -63,7 +63,6 @@ jobs: # wasm32-unknown-unknown builds, and even has the runtime fallback for # unsupported threading, but we don't have an environment to execute in. - # wasm32-wasi can test the fallback by running in wasmtime. wasm: name: WebAssembly (standalone) runs-on: ubuntu-latest From bad4be544ab86e5e8ca37da17a4ca0c8c52502f4 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Fri, 12 Jan 2024 19:21:16 +0000 Subject: [PATCH 09/12] Add wasi to the needs list --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 358bc6f4f..76575dab1 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -111,6 +111,6 @@ jobs: done: name: Complete runs-on: ubuntu-latest - needs: [check, test, demo, i686, wasm, fmt] + needs: [check, test, demo, i686, wasm, wasi, fmt] steps: - run: exit 0 From 53c18ffd313337078076495bbaf62b5772efd5c0 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Mon, 15 Jan 2024 17:53:28 +0000 Subject: [PATCH 10/12] Add explicit web_spin_lock feature --- Cargo.toml | 11 ++++++++--- rayon-core/Cargo.toml | 10 ++++++++-- rayon-core/src/lib.rs | 12 ++---------- src/iter/par_bridge.rs | 12 ++---------- 4 files changed, 20 insertions(+), 25 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index cc7a69e30..1806525e3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,15 +20,20 @@ exclude = ["ci"] [dependencies] rayon-core = { version = "1.12.0", path = "rayon-core" } - -[target.'cfg(all(target_arch = "wasm32", target_os = "unknown", target_feature = "atomics"))'.dependencies] -wasm_sync = "0.1.0" +wasm_sync = { version = "0.1.0", optional = true } # This is a public dependency! [dependencies.either] version = "1.0" default-features = false +[features] +# This feature switches to a spin-lock implementation on the browser's +# main thread to avoid the forbidden `atomics.wait`. +# +# Only useful on the `wasm32-unknown-unknown` target. +web_spin_lock = ["wasm_sync", "rayon-core/web_spin_lock"] + [dev-dependencies] rand = "0.8" rand_xorshift = "0.3" diff --git a/rayon-core/Cargo.toml b/rayon-core/Cargo.toml index f8793352a..41bdc960c 100644 --- a/rayon-core/Cargo.toml +++ b/rayon-core/Cargo.toml @@ -19,9 +19,15 @@ categories = ["concurrency"] [dependencies] crossbeam-deque = "0.8.1" crossbeam-utils = "0.8.0" +wasm_sync = { version = "0.1.0", optional = true } -[target.'cfg(all(target_arch = "wasm32", target_os = "unknown", target_feature = "atomics"))'.dependencies] -wasm_sync = "0.1.0" +[features] + +# This feature switches to a spin-lock implementation on the browser's +# main thread to avoid the forbidden `atomics.wait`. +# +# Only useful on the `wasm32-unknown-unknown` target. +web_spin_lock = ["wasm_sync"] [dev-dependencies] rand = "0.8" diff --git a/rayon-core/src/lib.rs b/rayon-core/src/lib.rs index b9dfdf019..39df8a2c3 100644 --- a/rayon-core/src/lib.rs +++ b/rayon-core/src/lib.rs @@ -103,18 +103,10 @@ pub use self::thread_pool::current_thread_index; pub use self::thread_pool::ThreadPool; pub use self::thread_pool::{yield_local, yield_now, Yield}; -#[cfg(not(all( - target_arch = "wasm32", - target_os = "unknown", - target_feature = "atomics" -)))] +#[cfg(not(feature = "web_spin_lock"))] use std::sync; -#[cfg(all( - target_arch = "wasm32", - target_os = "unknown", - target_feature = "atomics" -))] +#[cfg(feature = "web_spin_lock")] use wasm_sync as sync; use self::registry::{CustomSpawn, DefaultSpawn, ThreadSpawn}; diff --git a/src/iter/par_bridge.rs b/src/iter/par_bridge.rs index ac801af92..17bc15bf4 100644 --- a/src/iter/par_bridge.rs +++ b/src/iter/par_bridge.rs @@ -1,15 +1,7 @@ -#[cfg(not(all( - target_arch = "wasm32", - target_os = "unknown", - target_feature = "atomics" -)))] +#[cfg(not(feature = "web_spin_lock"))] use std::sync::Mutex; -#[cfg(all( - target_arch = "wasm32", - target_os = "unknown", - target_feature = "atomics" -))] +#[cfg(feature = "web_spin_lock")] use wasm_sync::Mutex; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; From f320d438f1e89636385f1dd4a762d0e1cf21c267 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Mon, 15 Jan 2024 17:55:12 +0000 Subject: [PATCH 11/12] Enable feature on CI --- .github/workflows/ci.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 76575dab1..16154bc35 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -71,6 +71,7 @@ jobs: include: - toolchain: stable - toolchain: nightly + cargoflags: --features web_spin_lock rustflags: -C target-feature=+atomics,+bulk-memory,+mutable-globals env: RUSTFLAGS: ${{ matrix.rustflags }} @@ -80,7 +81,7 @@ jobs: with: toolchain: ${{ matrix.toolchain }} targets: wasm32-unknown-unknown - - run: cargo build --verbose --target wasm32-unknown-unknown + - run: cargo build --verbose --target wasm32-unknown-unknown ${{ matrix.cargoflags }} # wasm32-wasi can test the fallback by running in wasmtime. wasi: From 97e7d776d3904103827ac0f7bf07926c62cfe1a5 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Mon, 15 Jan 2024 18:51:06 +0000 Subject: [PATCH 12/12] Apply suggestions from code review Co-authored-by: Josh Stone --- Cargo.toml | 2 +- rayon-core/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1806525e3..c2ee04bd1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,7 @@ default-features = false # main thread to avoid the forbidden `atomics.wait`. # # Only useful on the `wasm32-unknown-unknown` target. -web_spin_lock = ["wasm_sync", "rayon-core/web_spin_lock"] +web_spin_lock = ["dep:wasm_sync", "rayon-core/web_spin_lock"] [dev-dependencies] rand = "0.8" diff --git a/rayon-core/Cargo.toml b/rayon-core/Cargo.toml index 41bdc960c..b3de9270f 100644 --- a/rayon-core/Cargo.toml +++ b/rayon-core/Cargo.toml @@ -27,7 +27,7 @@ wasm_sync = { version = "0.1.0", optional = true } # main thread to avoid the forbidden `atomics.wait`. # # Only useful on the `wasm32-unknown-unknown` target. -web_spin_lock = ["wasm_sync"] +web_spin_lock = ["dep:wasm_sync"] [dev-dependencies] rand = "0.8"