-
Notifications
You must be signed in to change notification settings - Fork 2k
[wasm] Fix wasm backend not starting any threads #4957
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.
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
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
ef322ce
Fix threaded wasm not creating any workers
mattsoulanille c9dfebd
Remove wasm workers when dispose is called
mattsoulanille 6b65163
Merge branch 'master' into wasm_fix
mattsoulanille c81c482
Set pool size to navigator.hardwareConcurrency
mattsoulanille f276012
Merge branch 'wasm_fix' of github.com:mattsoulanille/tfjs into wasm_fix
mattsoulanille e29ea61
Separate cc_binary for simd
mattsoulanille 4c1369b
Compile threaded with '-pthread' copt
mattsoulanille 58930a4
Merge branch 'master' into wasm_fix
mattsoulanille 7b40891
Pass wasm module to emscripten
mattsoulanille f5754f9
Fix wasm paths for overrides map test
mattsoulanille 3deb52e
Merge branch 'wasm_fix' of github.com:mattsoulanille/tfjs into wasm_fix
mattsoulanille f0a17cb
Merge branch 'master' into wasm_fix
mattsoulanille File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,18 @@ KERNELS_WITH_KEEPALIVE = glob( | |
| exclude = ["**/*_test.cc"], | ||
| ) | ||
|
|
||
| BASE_LINKOPTS = [ | ||
| "-s ALLOW_MEMORY_GROWTH=1", | ||
| "-s DEFAULT_LIBRARY_FUNCS_TO_INCLUDE=[]", | ||
| "-s DISABLE_EXCEPTION_CATCHING=1", | ||
| "-s FILESYSTEM=0", | ||
| "-s EXIT_RUNTIME=0", | ||
| "-s EXPORTED_FUNCTIONS='[\"_malloc\", \"_free\"]'", | ||
| "-s EXTRA_EXPORTED_RUNTIME_METHODS='[\"cwrap\"]'", | ||
| "-s MODULARIZE=1", | ||
| "-s MALLOC=emmalloc", | ||
| ] | ||
|
|
||
| # This build rule generates tfjs-backend-wasm.{js,wasm}. | ||
| # | ||
| # The ".js" at the end of the build name is significant because it determines | ||
|
|
@@ -19,17 +31,36 @@ KERNELS_WITH_KEEPALIVE = glob( | |
| cc_binary( | ||
| name = "tfjs-backend-wasm.js", | ||
| srcs = ["backend.cc"] + KERNELS_WITH_KEEPALIVE, | ||
| linkopts = [ | ||
| "-s ALLOW_MEMORY_GROWTH=1", | ||
| "-s DEFAULT_LIBRARY_FUNCS_TO_INCLUDE=[]", | ||
| "-s DISABLE_EXCEPTION_CATCHING=1", | ||
| "-s FILESYSTEM=0", | ||
| "-s EXIT_RUNTIME=0", | ||
| "-s EXPORTED_FUNCTIONS='[\"_malloc\", \"_free\"]'", | ||
| "-s EXTRA_EXPORTED_RUNTIME_METHODS='[\"cwrap\"]'", | ||
| "-s MODULARIZE=1", | ||
| linkopts = BASE_LINKOPTS + [ | ||
| "-s EXPORT_NAME=WasmBackendModule", | ||
| ], | ||
| deps = [ | ||
| ":all_kernels", | ||
| ":backend", | ||
| ], | ||
| ) | ||
|
|
||
| cc_binary( | ||
| name = "tfjs-backend-wasm-simd.js", | ||
| srcs = ["backend.cc"] + KERNELS_WITH_KEEPALIVE, | ||
| linkopts = BASE_LINKOPTS, | ||
| deps = [ | ||
| ":all_kernels", | ||
| ":backend", | ||
| ], | ||
| ) | ||
|
|
||
| cc_binary( | ||
| name = "tfjs-backend-wasm-threaded-simd.js", | ||
| srcs = ["backend.cc"] + KERNELS_WITH_KEEPALIVE, | ||
| linkopts = BASE_LINKOPTS + [ | ||
| "-s EXPORT_NAME=WasmBackendModuleThreadedSimd", | ||
| "-s MALLOC=emmalloc", | ||
| "-s USE_PTHREADS=1", | ||
| "-s PROXY_TO_PTHREAD=1", | ||
| # Many x86-64 processors have 2 threads per core, so we divide by 2. | ||
| "-s PTHREAD_POOL_SIZE=" + | ||
| "'Math.min(4, Math.max(1, (navigator.hardwareConcurrency || 1) / 2))'", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it doable that we register a flag |
||
| ], | ||
| deps = [ | ||
| ":all_kernels", | ||
|
|
@@ -44,13 +75,13 @@ wasm_cc_binary( | |
|
|
||
| wasm_cc_binary( | ||
| name = "tfjs-backend-wasm-simd", | ||
| cc_target = ":tfjs-backend-wasm.js", | ||
| cc_target = ":tfjs-backend-wasm-simd.js", | ||
| simd = True, | ||
| ) | ||
|
|
||
| wasm_cc_binary( | ||
| name = "tfjs-backend-wasm-threaded-simd", | ||
| cc_target = ":tfjs-backend-wasm.js", | ||
| cc_target = ":tfjs-backend-wasm-threaded-simd.js", | ||
| simd = True, | ||
| threads = "emscripten", | ||
| ) | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the Simd module need a separate instance too or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The simd module doesn't need a separate interface because its API is the same as the non-simd API. The threaded simd module, however, does need a separate interface because it now has the
PThread.terminateAllThreadsfunction exposed (via the type declarations).