Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Threaded SIMD tests for the wasm backend crash the browser #4796

Closed
mattsoulanille opened this issue Mar 9, 2021 · 3 comments · Fixed by #4957
Closed

Threaded SIMD tests for the wasm backend crash the browser #4796

mattsoulanille opened this issue Mar 9, 2021 · 3 comments · Fixed by #4957
Assignees
@mattsoulanille
Labels
comp:wasm type:bug Something isn't working

Comments

@mattsoulanille
Copy link
Member

mattsoulanille commented Mar 9, 2021 •
edited
Loading

Please make sure that this is a bug. As per our
GitHub Policy,
we only address code/doc bugs, performance issues, feature requests and
build/installation issues on GitHub. tag:bug_template

System information

  • Have I written custom code (as opposed to using a stock example script provided in TensorFlow.js): 3.2
  • OS Platform and Distribution (e.g., Linux Ubuntu 16.04): Debian rodete
  • Mobile device (e.g. iPhone 8, Pixel 2, Samsung Galaxy) if the issue happens on mobile device:
  • TensorFlow.js installed from (npm or script link):
  • TensorFlow.js version (use command below):
  • Browser version: Chrome ~88
  • Tensorflow.js Converter Version:

Describe the current behavior
Threaded SIMD tests for the wasm backend create a new webworker for each test, crashing the page with hundreds of webworkers.

Describe the expected behavior
Each test does not have its own webworker, or webworkers are cleaned up after the test is finished.

Standalone code to reproduce the issue
Provide a reproducible test case that is the bare minimum necessary to generate
the problem. If possible, please share a link to Colab/CodePen/any notebook.

Other info / logs Include any logs or source code that would be helpful to
diagnose the problem. If including tracebacks, please include the full
traceback. Large logs and files should be attached.
@mattsoulanille mattsoulanille added type:bug Something isn't working comp:wasm labels Mar 9, 2021
@mattsoulanille mattsoulanille self-assigned this Mar 9, 2021
@mattsoulanille mattsoulanille mentioned this issue Mar 9, 2021
Merged
@mattsoulanille
Copy link
Member Author

Bundling all tests (with e.g. esbuild) does not fix this. While test files are no longer individually requested from Karma, each test still re-initializes the wasm backend (requesting the .wasm file each time) and creates a new webworker.

@carrycooldude
Copy link
Contributor

Hey @mattsoulanille, Actually for test and fixed-width SIMD maybe there is some issues with Web Assembly , Can you help me out in this
image

@rthadur rthadur mentioned this issue Apr 14, 2021
Closed
@mattsoulanille mattsoulanille mentioned this issue Apr 19, 2021
Merged
mattsoulanille added a commit that referenced this issue Apr 22, 2021
@mattsoulanille
[wasm] Fix wasm backend not starting any threads (#4957)
85c116a 
Without the linker option `PTHREAD_POOL_SIZE` set, the wasm backend does not create any webworkers and spins forever waiting for them to be created (crashing the browser). This PR sets `PTHREAD_POOL_SIZE` to `Math.min(4, Math.max(1, (navigator.hardwareConcurrency || 1) / 2))`, meaning it will use half of the logical cores available up to a max of 4 (or only 1 if `navigator.hardwareConcurrency` is not available). See [this comment](emscripten-core/emscripten#10263 (comment)) for details on why this works. This option apparently supersedes the value passed to [`pthreadpool_create` in `backend.cc`](https://github.com/tensorflow/tfjs/blob/c9dfebddfa34f531d2f0c363b6ea574a9fe9745d/tfjs-backend-wasm/src/cc/backend.cc#L65-L66), but its current setting should match the value computed in the `.cc` file. Fixes #4932 

This PR also fixes a bug where webworkers created by the wasm backend were not removed when `dispose` is called. Fixes #4796.

Fixes #4934 as well.
@google-ml-butler
Copy link

Are you satisfied with the resolution of your issue?
Yes
No

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mattsoulanille mattsoulanille

Labels
comp:wasm type:bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[wasm] Fix wasm backend not starting any threads mattsoulanille/tfjs
2 participants
@mattsoulanille @carrycooldude