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

WASM_WORKERS=2 documentation #21609

Open
msqr1 opened this issue Mar 25, 2024 · 14 comments · May be fixed by #21614
Open

WASM_WORKERS=2 documentation #21609

msqr1 opened this issue Mar 25, 2024 · 14 comments · May be fixed by #21614

Comments

@msqr1
Copy link

msqr1 commented Mar 25, 2024

What is that mode, and what does it do? Reading the code, I think it just merges the wasm worker into the main file. Also, if pthreads are allowed with SINGLE_FILE, why wouldn't wasm workers be?

@sbc100
Copy link
Collaborator

sbc100 commented Mar 25, 2024

pthreads doesn't currently work with -sSINGLE_FILE .. even though you don't see a compiler error, it still creates the worker as a seperate file, doesn't it?

Regarding WASM_WORKERS=2.. it doesn't look like this is well documented. I don't actually know what that wasn't just -sWASM_WORKERS + -sSINGLE_FILE. @juj do you remember?

@msqr1
Copy link
Author

msqr1 commented Mar 25, 2024

See, the name SINGLE_FILE is weird because all it does is merging the wasm into the main js. It should have been named MERGE_WASM instead. It does not create a big bundle where JS'ses got merged. So understanding it this way, both of them should work with pthread, right? I mean pthread and SINGLE_FILE still embed the wasm as base64 string as intended (even if it doesn't make a single file).

@sbc100
Copy link
Collaborator

sbc100 commented Mar 25, 2024

Yes, if you define SINGLE_FILE as simply meaning embed-wasm-in-js. But I think a lot of SINGLE_FILE users actually do want just a single file so that they don't need to host more than one file, or even run a server at all. So I think logically it would make sense for SINGLE_FILE to imply that the worker.js is also embedded. But I don't think we have a way to do that for pthreads today.

@msqr1
Copy link
Author

msqr1 commented Mar 26, 2024

I think we should change the name, and leave -sSINGLE_FILE for future implementations. This has been deceiving me left and right. Btw, I think a way to embed these stuff is with an object URL, like so:

var workerURL = URL.createObjectURL(new Blob(['(',
  (() => {
    // Some worker code
  }).toString(),
')()'], {type : "text/javascript"}))

and then use the URL as normal. This will enable minifiers like closure compiler to actually understand and optimize with custom levels. This is not my idea: https://stackoverflow.com/questions/5408406/web-workers-without-a-separate-javascript-file (below accepted answer)

@juj
Copy link
Collaborator

juj commented Mar 26, 2024

pthreads doesn't currently work with -sSINGLE_FILE .. even though you don't see a compiler error, it still creates the worker as a seperate file, doesn't it?

I see that emcc test\hello_world.c -pthread -sSINGLE_FILE would work, and run correctly, since it doesn't attempt to spawn any threads. But if attempting to launch threads, emcc test\pthread\hello_thread.c -pthread -sSINGLE_FILE , then execution fails. Posted #21617 to manage this incompatibility.

Regarding WASM_WORKERS=2.. it doesn't look like this is well documented. I don't actually know what that wasn't just -sWASM_WORKERS + -sSINGLE_FILE. @juj do you remember?

Posted a fix in #21614.

WASM_WORKERS=2 build mode embeds the Wasm Worker bootstrap script into the main .html file, so a separate a.ww.html file is not needed.

It would be appealing to think that we could support SINGLE_FILE+WASM_WORKERS just by making SINGLE_FILE imply WASM_WORKERS=2.

But that does not quite work, because Wasm Workers cannot importScripts() the generated JS file since it then resides inside the main .html file.

It would be possible to be bypassed by programmatically extracting the <script> tag from inside the single .html file, and then eval()ing that into the Worker context.

If someone would like to do the gymnastics I think we could add that support. But unfortunately might defeat the purpose for many users because it would need to rely on unsafe-eval.

For example, ads CDNs like to have a requirement of "single distributed file only", but they also like to have a requirement "no use of eval() or other dynamic execution", so it wouldn't enable that use case at least.

@msqr1
Copy link
Author

msqr1 commented Mar 26, 2024

I think a way to embed these stuff is with an object URL, like so:

var workerURL = URL.createObjectURL(new Blob(['(',
  (() => {
    // Some worker code
  }).toString(),
')()'], {type : "text/javascript"}))

and then use the URL as normal. This will enable minifiers like closure compiler to actually understand and optimize with custom levels. This is not my idea: https://stackoverflow.com/questions/5408406/web-workers-without-a-separate-javascript-file (below accepted answer)

Thoughs on this?

@sbc100
Copy link
Collaborator

sbc100 commented Mar 26, 2024

I think a way to embed these stuff is with an object URL, like so:

var workerURL = URL.createObjectURL(new Blob(['(',
  (() => {
    // Some worker code
  }).toString(),
')()'], {type : "text/javascript"}))

and then use the URL as normal. This will enable minifiers like closure compiler to actually understand and optimize with custom levels. This is not my idea: https://stackoverflow.com/questions/5408406/web-workers-without-a-separate-javascript-file (below accepted answer)

Thoughs on this?

I assume you are talking about making this work for pthreads? I don't see why this wouldn't work. Do you need this feature? Would you be interested in creating a PR? Would you like to open separate bug for this feature request?

@sbc100
Copy link
Collaborator

sbc100 commented Mar 26, 2024

Oh wait, we already have an issue for that #17547. Assuming that is what you want, lets continue this conversation there.

@msqr1
Copy link
Author

msqr1 commented Mar 27, 2024

Ohk

@juj
Copy link
Collaborator

juj commented Mar 27, 2024

I think a way to embed these stuff is with an object URL, like so:

var workerURL = URL.createObjectURL(new Blob(['(',
  (() => {
    // Some worker code
  }).toString(),
')()'], {type : "text/javascript"}))

Thoughs on this?

This is exactly what -sWASM_WORKERS=2 build mode is. It embeds the contents of the .ww.js file in an Object URL.

@VirtualTim
Copy link
Collaborator

This is similar to what I looked into ~5 years ago (#9796), but I abandoned it as it had a significant negative performance impact. Plus it didn't fix my initial issue.

If WASM_WORKERS=2 actually solves this, that issue should probably be closed.

@msqr1
Copy link
Author

msqr1 commented Mar 27, 2024

Wait, if we use the same approach we will get the same issue. Also, did you find out the main step where it takes long? Is it URL.createObjectURL?

@sbc100
Copy link
Collaborator

sbc100 commented Mar 27, 2024

I don't think we need to worry too much about the performance here for the initial version. Anyone who is using SINGLE_FILE is already paying a certain performance penalty because the wasm file is encoded as base64 as a JS string. The point of SINGLE_FILE is to put everything into just one file, and that can have a performance impact.

I suggest we start by getting something that works and then we can iterate to make it faster of folks notice a slowdown.

@sbc100
Copy link
Collaborator

sbc100 commented Mar 27, 2024

Also I think its possible that the approach suggested by @RReverser in #9796 could actually be a performance and complexity win.

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 a pull request may close this issue.

4 participants