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

Compiled code using pthreads breaks Vite bundler #22394

Open
jamsinclair opened this issue Aug 18, 2024 · 11 comments
Open

Compiled code using pthreads breaks Vite bundler #22394

jamsinclair opened this issue Aug 18, 2024 · 11 comments

Comments

@jamsinclair
Copy link

jamsinclair commented Aug 18, 2024

Description

When code using threads is compiled with Emscripten and then bundled with Vite it does not bundle and throws the following Vite error:

[vite:worker-import-meta-url] Vite is unable to parse the worker options as the value is not static.To ignore this error, please use /* @vite-ignore */ in the worker options.
file

Vite has required static worker options for some time, over 2 years. Recent changes to Emscripten have made output code incompatible with the Vite bundler.

Version of emscripten/emsdk:

Affects versions >= v3.1.58

Reproduction:
I have created a repository at https://github.com/jamsinclair/emscripten-worker-options-vite-reproduction which show cases the problem.

Potential Remedy:

Refactor library_pthread.js to use static values for the worker options. It'll be more verbose, but, it'll make the code compatible with Vite and maintain compatibility with other bundlers.

@jamsinclair
Copy link
Author

Vite users could potentially write a simple rollup or vite plugin to work around this. Although, if we could fix this in Emscripten itself, that would be amazing 🙏.

I know emscripten is in a tricky spot trying to support bundlers and their different nuances 😅

@x0YYYY0
Copy link

x0YYYY0 commented Aug 21, 2024

I have run into this issue as well. Thank you for posting about it. This is also unfortunate for those of us who are looking to use just shared memory, as the pthreads build option is the most comprehensive build option for that use case.

@sbc100
Copy link
Collaborator

sbc100 commented Aug 21, 2024

Yup, we can/should fix this. I wonder if we can write a test for it so we don't regress again.

Kind of still that vite can't see that the options are actually static.. just declared in a non-escaping var right before I think.

@dr-matt
Copy link

dr-matt commented Oct 18, 2024

I've also run into this issue recently. Is there currently a workaround?

@zamfofex-dummy
Copy link

You can just use version 3.1.57, and it should work.

@sbc100
Copy link
Collaborator

sbc100 commented Oct 18, 2024

We should fix this. I'll try to add some tests to emscripten so that we don't regress again too.

@dr-matt
Copy link

dr-matt commented Oct 19, 2024

@zamfofex-dummy thank you, good to know that's the version just before the regression. Unfortunately it would be tricky to move back to that version, as we're relying on other more recent changes.

@sbc100 that would be fantastic. Interestingly, we have one use case that works fine, but adding -g2 to the build produces this error. Haven't dug into why yet, might be project-specific.

@dr-matt
Copy link

dr-matt commented Oct 29, 2024

Just checking in on this.

While a proper fix is being worked on and tested, is there a workaround for this issue aside from reverting to a previous version of emscripten? We've temporarily switched to a non-threaded build for development, but we're now reaching the point of needing threads enabled to move forward. @jamsinclair you mentioned a possible vite plugin - is that something you've had luck with, and if so, what does it do exactly?

@jamsinclair
Copy link
Author

@dr-matt I've scaffolded a quick attempt at a plugin that could patch this issue. Check this gist https://gist.github.com/jamsinclair/6ad148d0590291077a4ce389c2b274ea.

Let me know how you go with it 👍

@dr-matt
Copy link

dr-matt commented Oct 31, 2024

@jamsinclair thanks! After making some mods for typescript compatibility, I found that this doesn't work as intended in my case because the js file is already minified - so rather than workerOptions the variable to be inlined happens to be named b. The regex doesn't match and the substitution doesn't happen. I think I need to turn off the closure compiler, or at least somehow disable its minification step, when building the wasm library.

edit: but I see what is needed and how the plugin works, so that's enough to move us forward - much appreciated!

@kleisauke
Copy link
Collaborator

Interestingly, we have one use case that works fine, but adding -g2 to the build produces this error. Haven't dug into why yet, might be project-specific.

I think this is due to the Closure compiler (--closure 1) is automagically inlining the workerOptions variable. For example, I see this in the generated JS output:

function Pe(){var a=new Worker(new URL("vips-es6.js",import.meta.url),{type:"module",name:"em-pthread"});Le.push(a)}

-g2 always disables the use of the Closure compiler.

emscripten/tools/link.py

Lines 952 to 954 in e6ed9d3

if settings.DEBUG_LEVEL > 1 and options.use_closure_compiler:
diagnostics.warning('emcc', 'disabling closure because debug info was requested')
options.use_closure_compiler = False

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

No branches or pull requests

6 participants