GoogleChromeLabs / wasm-bindgen-rayon

An adapter for enabling Rayon-based concurrency on the Web with WebAssembly.
https://github.com/RReverser/wasm-bindgen-rayon
Apache License 2.0
404 stars 35 forks source link

Calling `init_thread_pool` off `rust` #12

Closed entropylost closed 3 years ago

entropylost commented 3 years ago

Would it be possible to run init_thread_pool on the rust side not on the Javascript side? Like

fn main() {
  spawn_local(JsFuture::from(init_thread_pool(4)));
}

?

RReverser commented 3 years ago

In theory you can, but I decided to go with JS API because:

  1. It adds a lot more overhead - wasm-bindgen-futures itself costs quite a bit in terms of size, and then you need various other bindings to spawn Workers, get number of threads etc. This adapter was intentionally designed to be as low-overhead in terms of both size and startup time as possible.
  2. There is no actual "main" function in wasm-bindgen libraries. You could use wasm_bindgen(start) but it doesn't wait for Promises which is absolutely critical for init-thread-pool. Or you could expose a named export with function that returns a Promise when init_thread_pool finished... and now we're back to square one, since this is exactly the same as existing init_thread_pool.
  3. For --target web in wasm-bindgen, you already need to initialize wasm-bindgen from JS anyway (via init()) so adding all this overhead just isn't worth it for the little gain, when you can instead just call both inits from JS together.
RReverser commented 3 years ago

Probably should also point out that your example specifically won't work. It's important to await (return execution to the event loop) so that the Workers can be actually spawned, before calling any other methods.

spawn_local by itself only schedules the creation of said Workers, so if you try to do this and then immediately call some other Rust functions that rely on Rayon, you'll get a deadlock.

entropylost commented 3 years ago

Hmm but what if I'm having the entirety of my code within an async function? Then could I do init_thread_pool().await?

RReverser commented 3 years ago

Yes, that would work - in that case see the comment before the last one. If you are okay with those tradeoffs in your own app, then yeah, you should be able to call it from Rust by converting JsValue returned from init_thread_pool to Promise -> JsFuture and then doing .await.

I only tried to explain why those tradeoffs were not ideal for the common use and why it's not something integrated in the core of the adapter.

entropylost commented 3 years ago

Okay. (Also init_thread_pool returns a Promise not a JsValue)

RReverser commented 3 years ago

Okay. (Also init_thread_pool returns a Promise not a JsValue)

Oh right, I changed that relatively recently hence forgot.

RReverser commented 3 years ago

Do you feel that this question has been answered / can this issue be closed?

entropylost commented 3 years ago

Sorry about that.

entropylost commented 3 years ago

It might be worth removing the doc(ignore) on the function though.

RReverser commented 3 years ago

It might be worth removing the doc(ignore) on the function though.

Right, it was hidden to discourage people using it directly from Rust and making sure that docs show actual docs with the JS usage... but I guess if someone (like you) starts relying on it, might as well keep it public.

RReverser commented 3 years ago

Btw you might be interested in https://github.com/seanmonstar/num_cpus/pull/103. I've sent it long time ago with no response, but this was exactly the usecase it's meant to help with.

entropylost commented 3 years ago

I just did window().unwrap().navigator().hardware_concurrency() as usize

RReverser commented 3 years ago

Yeah that works, especially if you don't need to do unit itself in a Worker. The PR above would be a bit more efficient and correct for reasons outlined in its description.