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

`init_thread_pool(0)` will freeze the worker with no error #20

Closed zxch3n closed 2 years ago

zxch3n commented 2 years ago

Expected behavior

Switching to optimal thread number or throwing an error.

RReverser commented 2 years ago

Adding an error makes sense, but I wonder how/why did you pass 0 in real-world code.

zxch3n commented 2 years ago

I thought the behavior would be the same as Rayon's num_threads API.

If num_threads is 0, or you do not call this function, then the Rayon runtime will select the number of threads automatically. At present, this is based on the RAYON_NUM_THREADS environment variable (if set), or the number of logical CPUs (otherwise).

And many other thread-related APIs also use 0 or -1 to select the number of threads automatically. No idea whether it's possible in the browser context though.

This behavior can be learned during development, but document or error messages can be very helpful.

And thanks for this tool! It's really amazing.

RReverser commented 2 years ago

Ah interesting.

I kind of wanted to keep number of threads manual to make sure that users don't get code bloat from automatic detection, so for now I'm leaning towards making this a clear error rather than automatic detection. I believe Rayon also said in their docs that default behaviour can change in future and users should pass explicit number of threads too.

Need to check how to do it without also significant code bloat though.

RReverser commented 2 years ago

If you want to send a PR to add such error, that would be welcome too btw.

zxch3n commented 2 years ago

Ok, I'll submit a PR~