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.

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

Combining wasm workers and pthreads leads to excessive worker creation #20279

Closed
dwz8 opened this issue Sep 17, 2023 · 7 comments
Closed

Combining wasm workers and pthreads leads to excessive worker creation #20279

dwz8 opened this issue Sep 17, 2023 · 7 comments
Assignees

Comments

@dwz8
Copy link

dwz8 commented Sep 17, 2023

I am using both pthreads and wasm workers, and I see more workers being created than necessary.
The reason seems to be the following:

PThread.init checks for ENVIRONMENT_IS_PTHREAD first, in all other cases Pthread.initMainThread is called. To me it looks like there should be an additional check for ENVIRONMENT_IS_WASM_WORKER in the generated js:

 init: function() {
  PThread.debugInit();
  if (ENVIRONMENT_IS_PTHREAD || ENVIRONMENT_IS_WASM_WORKER) {
   PThread.initWorker();
  } else {
   PThread.initMainThread();
  }
 },

The current init function leads to initMainThread being called for each wasm worker, which in turn will call PThread.init to allocate the number of workers specified by PTHREAD_POOL_SIZE multiple times..
It seems to work correctly with the suggested change, however, I am not familiar enough with the internals of Emscripten to make a judgement.

Version of emscripten/emsdk: 3.1.46

Command line in full:
emcc ./src/main.c ...list if source files
-I ./include/ ^
-g3 ^
--source-map-base ./ ^
-gsource-map ^
-s ALLOW_MEMORY_GROWTH=1 ^
-s ENVIRONMENT=web,worker ^
--shell-file ./index_template.html ^
-s SUPPORT_ERRNO=0 ^
-s MODULARIZE=1 ^
-sMALLOC=emmalloc ^
-sWASM_WORKERS ^
-sOFFSCREEN_FRAMEBUFFER ^
-pthread ^
-s PTHREAD_POOL_SIZE=2 ^
-s "EXPORT_NAME='wasmMod'" ^
-s EXPORTED_FUNCTIONS="['_malloc','_free','_main']" ^
-s EXPORTED_RUNTIME_METHODS="['cwrap','UTF16ToString','UTF8ToString','stringToUTF8','allocateUTF8','AsciiToString']" ^
-o index.html

@sbc100
Copy link
Collaborator

sbc100 commented Sep 18, 2023

Assigning @juj initially. If you would rather a not do this @juj please let me know and feel to un-assign yourself.

@simonbuehler
Copy link

simonbuehler commented Feb 1, 2024

i experience the same issue:

user@DESKTOP-68JUJRU:~/opencv/emsdk$ python ./opencv/platforms/js/build_js.py  \
--cmake_option="-DCMAKE_TOOLCHAIN_FILE=/home/user/opencv/emsdk/upstream/emscripten/cmake/Modules/Platform/Emscripten.cmake" \
--cmake_option="-DCMAKE_CROSSCOMPILING_EMULATOR=/home/user/opencv/emsdk/node/16.20.0_64bit/bin/node"  \
build_wasm \
--clean_build_dir  \
--build_wasm  \
--simd \
--threads \
--build_flags="-s EXPORT_ES6=1 " \
--disable_single_file

so that does execute this finally:

/usr/bin/cmake -E cmake_link_script CMakeFiles/opencv_js.dir/link.txt --verbose=1
/home/user/opencv/emsdk/upstream/emscripten/em++ -s WASM=1 -s USE_PTHREADS=1 -s PTHREAD_POOL_SIZE=4 -msimd128 -s EXPORT_ES6=1    -fsigned-char -W -Wall -Wreturn-type -Wnon-virtual-dtor -Waddress -Wsequence-point -Wformat -Wformat-security -Wmissing-declarations -Wmissing-prototypes -Wstrict-prototypes -Wundef -Winit-self -Wpointer-arith -Wshadow -Wsign-promo -Wuninitialized -Winconsistent-missing-override -Wno-delete-non-virtual-dtor -Wno-unnamed-type-template-args -Wno-comment -Wno-deprecated-enum-enum-conversion -Wno-deprecated-anon-enum-enum-conversion -fdiagnostics-show-option -pthread -Qunused-arguments -ffunction-sections -fdata-sections  -fvisibility=hidden -fvisibility-inlines-hidden -O3 -DNDEBUG  -DNDEBUG   -Wl,--gc-sections  --memory-init-file 0 -s TOTAL_MEMORY=128MB -s WASM_MEM_MAX=4GB -s ALLOW_MEMORY_GROWTH=1 -s MODULARIZE=1 -s EXPORT_NAME="'cv'" -s DEMANGLE_SUPPORT=1 -s FORCE_FILESYSTEM=1 --use-preload-plugins --bind --post-js /home/user/opencv/emsdk/opencv/modules/js/src/helpers.js  -Wno-missing-prototypes @CMakeFiles/opencv_js.dir/objects1.rsp -o ../../bin/opencv_js.js @CMakeFiles/opencv_js.dir/linklibs.rsp

the idea with ENVIRONMENT_IS_WASM_WORKER didn't work as its undefined, any hint how to fix this?

@sbc100
Copy link
Collaborator

sbc100 commented Feb 1, 2024

@simonbuehler looks like you are just using pthreads and not wasm workers, is that right? If so I don't think this is the same issue.

@simonbuehler
Copy link

@sbc100 oops you are right, my bad - the symptoms are the same though.

@simonbuehler
Copy link

i think i found the reason: when adding a additional worker between the vue component and the opencv workers so the UI stays responsive like this:

created() {
    this.worker = new Worker(
        new URL( "/src/js/mertensWorker.js", import.meta.url),
        { type: "module" }
    );
    this.worker.addEventListener("message", this.handleWorkerMessage);
},
unmounted() {
    if (this.worker) {
        this.worker.terminate();
    }
},

and in there :

console.log("WebWorker initialized");

// Importieren von OpenCV.js
import cv from "./opencv";
console.log(cv.getBuildInformation());

//using cv ...

initially 4 workers are created but on invocation of the first cv method the additional ones get created, strange.
Should I open an extra issue for this @sbc100 ?

@sbc100
Copy link
Collaborator

sbc100 commented Feb 2, 2024

Sure, sounds like a seprate issue. If you build opencv with -sPTHREAD_POOL_SIZE=4 though it completely expected the 4 workers would be created when you load the module. Thats what -sPTHREAD_POOL_SIZE=4 does.

@juj
Copy link
Collaborator

juj commented Feb 7, 2024

Thanks, this is a great observation. Posted PR #21290 , I think that should fix this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants