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

Emit WebAssembly by default instead of asm.js #6168

Closed
kripken opened this issue Jan 31, 2018 · 9 comments
Closed

Emit WebAssembly by default instead of asm.js #6168

kripken opened this issue Jan 31, 2018 · 9 comments
Assignees

Comments

@kripken
Copy link
Member

kripken commented Jan 31, 2018

Now that all major browsers support wasm, and wasm output has been pretty stable for a while, maybe it's a good time to change the default output from asm.js to wasm?

Benefits:

  • People should be using wasm since it's smaller&faster, unless they have a specific reason to need asm.js. So wasm by default makes more sense.
  • Changing to wasm by default would move our test suite to focus mostly on that (e.g. right now almost all browser tests are asm.js). That would lead to better coverage of wasm, and also the wasm tests are faster.

Stuff to think about:

  • This is orthogonal to our wasm support switching to the LLVM wasm backend by default Switching to use upstream LLVM backend by default #5488. So we could in theory do asm.js=>wasm earlier or later. Doing it earlier might make sense as it could be done sooner. Also that way means a more gradual change (2 big changes to the default output instead of one huge one).
  • Aside from the main work to switch the default, this would require a lot of updates to our docs and tests, and overall we should make an effort to minimize confusion for users.
@dschuff
Copy link
Member

dschuff commented Jan 31, 2018

Yeah I think we should do this, for the reasons you said. Agreed that there's no need to block on the wasm backend by default transition.

@curiousdannii
Copy link
Contributor

I'd say this would warrant bumping the version number at a higher than patch level.

What's the current state of loading wasm files in arbitrary/relative directories? I think #5368 and/or #5484 should be resolved first.

@eclecticdave
Copy link
Contributor

eclecticdave commented Feb 1, 2018

AIUI threads/atomic operations are not currently available in wasm, so would we default to wasm unless "-s USE_PTHREADS" is specified, or would it better to have to specifically state asmjs in this case (and have an error thrown if you don't)?

Edit: specifically specify ... huh! :-)

@eclecticdave
Copy link
Contributor

eclecticdave commented Feb 1, 2018

Sorry, just seen https://github.com/kripken/emscripten/wiki/Pthreads-with-WebAssembly. So, probably not a problem - I imagine it'll be in release versions (of browsers) by the time this lands (though possibly still behind a flag)

@ad8e
Copy link
Contributor

ad8e commented Feb 1, 2018

According to https://caniuse.com/#feat=wasm, 69% of browsers, mobile and desktop combined, support WebAssembly. Most of the incompatibility is in mobile browsers. I have no comment on which default is better.

@kripken
Copy link
Member Author

kripken commented Feb 1, 2018

I'd say this would warrant bumping the version number at a higher than patch level.

Agreed. We should do that more often in general too. I'll wrote a doc for discussion, #6174.

What's the current state of loading wasm files in arbitrary/relative directories?

Those two PRs appear to be still open - @juj, can you please make it a priority to resolve things there?

Otherwise, though, locateFile has been fixed to work for customizing loading the wasm file, which should handle many cases.

AIUI threads/atomic operations are not currently available in wasm

Yeah, and the same for SIMD - those two features are in JS (perhaps behind a flag due to Spectre...) but not in wasm yet. I don't think we should wait on that, but we should document and warn in the compiler about this (and then people can emit JS instead of wasm if they want those).

According to https://caniuse.com/#feat=wasm, 69% of browsers, mobile and desktop combined, support WebAssembly.

Thanks for the stat, good to have a concrete number. When doing this we should definitely document that.

@kripken kripken self-assigned this Feb 16, 2018
@kripken
Copy link
Member Author

kripken commented Apr 4, 2018

PR for this: #6419

@DesWurstes
Copy link

DesWurstes commented Jul 8, 2018

This issue can be closed now.

@kripken
Copy link
Member Author

kripken commented Jul 8, 2018

Yes, thanks @DesWurstes!

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