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

Make USE_PTHREADS work with MODULARIZE mode. #5016

Closed
wants to merge 1 commit into from

Conversation

bvibber
Copy link
Collaborator

@bvibber bvibber commented Mar 9, 2017

pthread-main.js was touching globals directly, which aren't
exposed as globals when the Module object is encapsulated
into a function constructor with MODULARIZE and EXPORT_NAME
options.

Modified pthread-main.js and library_pthread.js (and a little
modification to shell.js and jsifier.js) to use accessors for
a couple things, and to inject the initialized stuff for pthread
workers via options on the Module object instead of directly.

emcc.py is changed to support checking document.currentScript.src
at load time, outside the module wrapper constructor, and passes
the value in to the module via Module.currentScriptUrl; this is
overriding the document.currentScript.src check in shell.js.

Shouldn't break existing non-modular code, but does require that
pthread-main.js be updated when recompiling (as should happen
normally).

Fixes #5009

bvibber added a commit to bvibber/ogv.js that referenced this pull request Mar 10, 2017
Works, but currently halts the main thread :D
Gives a roughly 20-40% improvement on VP9 decode at
480p and 720p on MacBook Pro early 2015 (2 cores plus
hyperthreading for 4 worker threads).

Requires patched emscripten for modularize stuff
emscripten-core/emscripten#5016

todo:
* get the emscripten fixes upstream
* don't block main thread
* see if helps VP8 too
* do benchmarks
* autodetect etc
@bvibber
Copy link
Collaborator Author

bvibber commented Apr 20, 2017

Updated against current incoming branch.

@bvibber
Copy link
Collaborator Author

bvibber commented May 26, 2017

Updated against current incoming branch.

@kripken kripken requested a review from juj May 26, 2017 17:38
pthread-main.js was touching globals directly, which aren't
exposed as globals when the Module object is encapsulated
into a function constructor with MODULARIZE and EXPORT_NAME
options.

Modified pthread-main.js and library_pthread.js (and a little
modification to shell.js and jsifier.js) to use accessors for
a couple things, and to inject the initialized stuff for pthread
workers via options on the Module object instead of directly.

emcc.py is changed to support checking document.currentScript.src
at load time, outside the module wrapper constructor, and passes
the value in to the module via Module.currentScriptUrl; this is
overriding the document.currentScript.src check in shell.js.

Shouldn't break existing non-modular code, but does require that
pthread-main.js be updated when recompiling (as should happen
normally).

Used in ogv.js 1.4.x for experimental multithread builds.

Fixes emscripten-core#5009
@bvibber
Copy link
Collaborator Author

bvibber commented May 26, 2017

(and one last tweak for the latest merges)

@TannerRogalsky
Copy link
Contributor

Thanks for this, @Brion.

I have it working now but when I compiled with the default EXPORT_NAME, it looked like the worker threads were trying to spawn their own workers and were breaking. It might be nice to guard against that.

@kripken
Copy link
Member

kripken commented Sep 21, 2017

ping @juj

@kripken
Copy link
Member

kripken commented Oct 17, 2017

re-ping @juj

@juj
Copy link
Collaborator

juj commented Nov 10, 2017

Hey, sorry for the delay on this one. There's quite a few changes coming in to this, so I'll need to revise this somewhat to land. @Brion : what's your method of testing/using this? Do you have a specific setup, or just enabling the -s MODULARIZE linker flags?

@bvibber
Copy link
Collaborator Author

bvibber commented Nov 10, 2017

Yeah, it's a little messy and may need cleanup, I kind of hacked it until it worked. :)

I was doing ad-hoc testing with ogv.js's libvpx wrapper, which unfortunately has a lot of moving parts. I've disabled pthreads from the mainline ogv.js build process until wasm threading arrives but if you want to try it, the last version with the pthreads build included is commit 4b46eb44cf490fe561402d251595d2b8dd273e43.

Module source (linked to libvpx): https://github.com/brion/ogv.js/blob/master/src/c/ogv-decoder-video-vpx.c
Individual build script: https://github.com/brion/ogv.js/blob/master/buildscripts/compileOgvDecoderVideoVP9MT.sh so the relevant bits:

  -s USE_PTHREADS=1 \
  -s PTHREAD_POOL_SIZE=1 \
  -s NO_EXIT_RUNTIME=1 \
  -s EXPORT_NAME="'OGVDecoderVideoVP9MT'" \
  -s MODULARIZE=1 \

@bvibber
Copy link
Collaborator Author

bvibber commented Feb 7, 2019

Obsoleted by #7667 which does it better :)

@bvibber bvibber closed this Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants