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

Better WebAssembly integration #2099

Open
xtuc opened this issue Mar 29, 2018 · 26 comments
Open

Better WebAssembly integration #2099

xtuc opened this issue Mar 29, 2018 · 26 comments

Comments

@xtuc
Copy link

xtuc commented Mar 29, 2018

👋

I'm currently working on the WebAssembly integration in Webpack and I was wondering if Rollup would be interested into a first-class support for bundling wasm as well?

I know that https://github.com/rollup/rollup-plugin-wasm already exist but inline the wasm module into the bundle is not ideal. cc @jamen

In order to provide the best integration we want wasm to be part of the ES module graph, and treated as any other ES module. It has some issues at the moment, you can find more details here https://github.com/WebAssembly/esm-integration.

Rollup inlines the imports and fixes most of our integration issues!

I already experimented with a plugin, here's an example:

import("./test.wasm").then(({test}) => {
  test()
});

Complet example here https://github.com/xtuc/rollup-plugin-streaming-wasm/blob/master/example/src/

The dynamic import makes it more friendly for the developer (see old version).

@xtuc xtuc changed the title WebAssembly support Better WebAssembly integration Mar 29, 2018
@jamen
Copy link

jamen commented Mar 29, 2018

This would be great in my opinion.

I agree inlining the WebAssembly is not ideal. I had some vague idea about doing a similar thing in rollup-plugin-wasm, but had no idea how to approach it and I was missing some important details about the module graph. Seeing the WebAssembly/esm-integration proposal and your plugin, this is would be an improvement for sure.

@xtuc
Copy link
Author

xtuc commented Mar 29, 2018

What do you think about having first-class support vs a plugin? I'm sure that WebAssembly will become be more and more popular, a plugin could add some friction.

Also having an general view of the project is important because you can download/instantiate the wasm modules in parallel. It seems difficult to handle in a plugin.

I made several tools for WebAssembly, you can find them here https://github.com/xtuc/webassemblyjs. In the future we could enable DCE (based on the real usage in JS), AOT optimizations like inlining constants.

I would be happy to send a PR btw.

@guybedford
Copy link
Contributor

@xtuc thanks for opening this, your tools and approach would integrate with Rollup really well definitely. I've followed some of your work in the Webpack integration and your PR to start function rewriting for execution ordering was really nice to see too.

How would you see an initial integration working here? There's no doubt that this is the direction things should be going and contributions here would be very welcome. I'm just wondering how best to start to be practical today while ensuring a good foundation for the cross-format DCE work.

Transparent integration of wasm should definitely be supported in Rollup by default in future - whether through a bundled plugin or core itself. Help getting us there would be really amazing!

From a high level it feels like Rollup definitely should integrate this into core, but when I dive into the details I wonder if starting from plugin hooks might be a better proving ground for this work at least for now.

Some thoughts here:

  • Focusing on dynamic import as the mechanism sounds good to avoid the async issues, but then what would the experience around static imports be? Likely we would have to throw an error or similar (at least until transparent WASM imports start to become supported in Node and browsers), although the System module format can support WASM loading through the dev loader (and the production loader soon, I'm currently working on the streaming loading support)
  • Using just dynamic import, we lose a lot of the treeshaking benefits, as we don't currently have any export pruning detection for dynamic import usages at the moment. This is definitely something we should add though, and we'd definitely be interested in improving this support if it can open the door to WASM DCE.
  • That said, I wonder whether an alternative to sharing the tree shaking with plugins might be to open up a hook here like pruneUnusedExports(exportNames: string[]) where the plugin can then apply its own optimizations. This could help other plugins like CSS modules to remove dead CSS. I may be doing some work on the plugin hooks soon to ensure they all work well with the code splitting workflows, so it's a good time to be discussing this side too.
  • Rollup currently only emits a single file for a normal file build. It sounds like you're against inlining WASM binaries into JS, so outputting a JS + WASM file is a slightly different model to what we support now, but perhaps as code splitting workflows in Rollup are used more, WASM support can just be seen as part of that.

Do you see other potential benefits of core integration over direct export pruning? Would a plugin hook like the one suggested help to make progress here, or do you strongly feel a core integration is needed already?

@xtuc
Copy link
Author

xtuc commented Mar 30, 2018

I've followed some of your work in the Webpack integration and your PR to start function rewriting for execution ordering was really nice to see too.

😊 thanks.

How would you see an initial integration working here?

The initial integration is pretty straightforward I think. The plugin https://github.com/xtuc/rollup-plugin-streaming-wasm is a working prototype.

From a high level it feels like Rollup definitely should integrate this into core, but when I dive into the details I wonder if starting from plugin hooks might be a better proving ground for this work at least for now.

It's safer to do it as a plugin at the beginning and move it into the core later. I don't have enough context to see where would be the limitation of that approach.

Focusing on dynamic import as the mechanism sounds good to avoid the async issues, but then what would the experience around static imports be?

You can learn more about the different instantiation methods here: https://docs.google.com/document/d/1YB2fy3gMgy2NZcOVJhRGOxcM8bDAtC3nUizDRcPmE-I

Basically WebAssembly.instantiateStreaming() has the most benefits, one downside is that it's asynchronous. Note that Webpack throws when you use an ImportDeclaration with the module as source.

Using just dynamic import, we lose a lot of the treeshaking benefits

I wrote a static analyzer to collect the usage https://github.com/xtuc/wasm-dce/blob/feat-cli/src/used-exports.js. We can adapt it to the syntax we'll use here and I think that DCE will still be possible.

That said, I wonder whether an alternative to sharing the tree shaking with plugins might be to open up a hook here like pruneUnusedExports(exportNames: string[]) where the plugin can then apply its own optimizations

That's indeed an alternative, the @webassembly/dce offers a similar API where you pass the used exports and the binary. The resulting binary will be pruned.

Rollup currently only emits a single file for a normal file build. It sounds like you're against inlining

In the plugin I just wrote in the root but I guess that we can use the rollup.config.js to figure out the dist folder?

In addition to the document I linked above, inlining has the downside of having to manipulate/represent an arbitrary size binary (nothing prevents to have +100Mb).

Do you see other potential benefits of core integration over direct export pruning? [...]

The @webassemblyjs/dce is not readdy yet (I need to reimplement it using @webassemblyjs/wasm-edit). I would rather don't DCE for the initial integration.


Here's a well-explained comment WebAssembly/esm-integration#1 (comment) about the WASM<>ESM integration issues.

Rollup is actually not impacted because it inlines the imported values into the first module. Which means that every imported JS value is already evaluated and available!

I hope I answered all your questions clearly enough.

@guybedford
Copy link
Contributor

I wrote a static analyzer to collect the usage https://github.com/xtuc/wasm-dce/blob/feat-cli/src/used-exports.js. We can adapt it to the syntax we'll use here and I think that DCE will still be possible.

This is the sort of direction exactly. If you're interested in working on this problem in Rollup we're happy to assist.

In the plugin I just wrote in the root but I guess that we can use the rollup.config.js to figure out the dist folder?

In a single-file output you can read from the output.file or for a code splitting workflow from output.dir. The ongenerate doesn't have the chunk name for multiple chunks currently, as you'd probably want the web assembly file to be the same name as the js chunk or similar? So there's a few subtleties around these different output modes.

Rollup is actually not impacted because it inlines the imported values into the first module. Which means that every imported JS value is already evaluated and available!

Note this doesn't apply to externals or code splitting workflows where chunks are separated, although keeping WASM imports in the same chunk as the JS corresponding to a WASM file will likely be an important signal.

The @webassemblyjs/dce is not readdy yet (I need to reimplement it using @webassemblyjs/wasm-edit). I would rather don't DCE for the initial integration.

It could well be worth seeing if we can create something like these plugin hooks in the mean time for when it is.

I hope I answered all your questions clearly enough.

Certainly. It's worth making sure we're playing to Rollup's strengths in the use cases to find where we can have the highest impact here. Perhaps dynamic import export usage tracing + a plugin hook to your wasm plugin would be that shortest path of greatest impact to get movement here? Although it would depend on how ready your project is there. Otherwise let me know what you think is the best focus.

@xtuc
Copy link
Author

xtuc commented Mar 30, 2018

This is the sort of direction exactly. If you're interested in working on this problem in Rollup we're happy to assist.

Sure, in fact I have done it again here xtuc/webassemblyjs#223 during the day.

Note this doesn't apply to externals or code splitting workflows where chunks are separated, although keeping WASM imports in the same chunk as the JS corresponding to a WASM file will likely be an important signal.

That's a good point. We can consider every following and the wasm itself as a splitted bundle (which is the case with the dynamic import anyway?).

_

It seems that DCE is important for Rollup. I would personally not allow it for wasm currently, we just need to make clear that it's something experimental. I don't want to force you to introduce a new temporary API.

Although it would depend on how ready your project is there

Just to clarify, it's only the dce package that's not usable currently, we have a wasm-edit package that will replace it and it's already what Webpack is using.

@xtuc
Copy link
Author

xtuc commented Apr 3, 2018

If everything is clear to you, I'll continue my work in the plugin.

We could also setup a video call if needed.

@guybedford
Copy link
Contributor

@xtuc I've been thinking about this a bit further an in order to properly handle wasm imports and the rewriting of wasm imports, a core Rollup module type definitely seems like the best approach.

Also as soon as browsers support import 'x.wasm' (which seems to be moving fast now, so probably in the next few months), then we'd want Rollup code splitting to support this sort of thing, where JS files that have been bundled and renamed can still be referenced from wasm.

So I do think we should start integrating WASM into core now actually, or at least in the next couple of months or so. Then we can also start to align the architecture around the needs of tree shaking both formats together.

@lukastaegert @Rich-Harris let me know if you disagree that WASM should be integrated as a core WasmModule.ts construct?

@xtuc so, perhaps we can start planning how such an integration might work?

@lukastaegert
Copy link
Member

Admittedly I do not have a lot of experience with WASM yet. Is this a real use case that WASM files reference JS files? In the examples e.g. here, imports always seem to be passed to the WASM code by the JS code which means the WASM code does not need to be aware of how files are named and we maybe just need to extend the plugin API a little. But I would trust your judgement.

@guybedford
Copy link
Contributor

guybedford commented Apr 4, 2018

Yes, WASM files always need to import from a JS file that defines basically the "syscalls" for the WASM to properly be configured in the environment (eg things like printf -> console.log). The standard pattern here is a import { malloc } from './env.js' etc in the wasm output file.

@guybedford
Copy link
Contributor

Correction - env.js in the previous example.

@guybedford
Copy link
Contributor

The importObject used in the programmatic API is moving to using the module system itself - so this is the sort of stuff we can catch.

@xtuc
Copy link
Author

xtuc commented Apr 4, 2018

the rewriting of wasm imports

What do you mean? I don't think we need to rewrite them in Rollup.

import 'x.wasm'

Well if the user don't use any result value we can allow it and instantiate it asynchronously.

Unlike: import {exportFoo} from "x.wasm" which would require sync instantiation. I need to think about it more, i'm not sure how we can do it.

Note that in some cases a user might want to explicitly use the sync instantiation, for example in Node.

The importObject used in the programmatic API is moving to using the module system itself - so this is the sort of stuff we can catch.

Yes exactly, the ImportObject is more a dynamic linking phase. It happen to map well to ES module and removes boilerplate code from the user. You will find more information here WebAssembly/esm-integration#5

@guybedford
Copy link
Contributor

What do you mean? I don't think we need to rewrite them in Rollup.

Say you have a file node_modules/pkg/x.wasm that imports node_modules/pkg/env.js, then when running a rollup build we want to ensure that env.js is located in the output folder, and also that if using eg hash naming env.js import is renamed to env-[hash].js etc.

Unlike: import {exportFoo} from "x.wasm" which would require sync instantiation. I need to think about it more, i'm not sure how we can do it.

There is no way to support this apart from outputting import {exportFoo} from 'x.wasm', but after providing Rollup benefits to the rest of the build.

Note that SystemJS output can be made equivalently here that would then support the wasm instantiation through the system production loader. The AMD and CommonJS output cases wouldn't work though, without "inlining" the instantiation (in which case dynamic import is better suited anyway).

Perhaps we need a way to distinguish between cases where we have a "native wasm loader" we can use and cases where we need to inline a WASM instantiation process like you do in the plugin.

@guybedford
Copy link
Contributor

Trying to think how we might attach something here, although further suggestions welcome -

Here are some of the integration points:

As I try to think through this it does seem it's a new level of abstraction that needs to bump quite a few things. Accepted that the starting point will be hacky to get something going, and I'm happy to dive in myself to help find the right abstractions further to refactor towards something cleaner as we go.

I hope that gives at least some rough overview to start though.

@guybedford
Copy link
Contributor

(Also we should create a "wasm" branch for this work to happen off mainline I think until it stabilizes)

@guybedford
Copy link
Contributor

guybedford commented Apr 5, 2018

Ok I've created a wasm branch, and also based this off the typing changes in #2108 to avoid the conflicts when that merges. We can track any PR work against this branch and then merge aggressively (into the wasm branch) to get some experimental collaboration going on this hopefully.

@xtuc
Copy link
Author

xtuc commented Apr 6, 2018

I started a PR here #2113.

The single-file build has preRender and render called from https://github.com/rollup/rollup/blob/master/src/rollup/index.ts#L346, which would need to be adjusted to handle the binary output scenario. Perhaps OutputChunk at https://github.com/rollup/rollup/blob/master/src/rollup/index.ts#L248 can become { code: string | Buffer } as the simplest start here.

Actually, that means you have the wasm file as entrypoint? Currently you JS to require it so I don't think that's needed.

@shellscape
Copy link
Contributor

@xtuc I'm just stepping into this and playing catch up. Are there any blockers we're looking at that are holding this up?

@xtuc
Copy link
Author

xtuc commented Jul 28, 2018

Thanks @shellscape we have some points in the PR thread: #2113, mostly about the integration of binary modules into the module graph.

@DavidPeicho
Copy link

Hi guys, It looks like you did a nice job. What is the current status of this feature?

@brettz9
Copy link

brettz9 commented Jun 17, 2019

FWIW, Node 12 (12.4.0) now has import support for Wasm, albeit behind a flag: https://nodejs.org/api/esm.html#esm_experimental_wasm_modules

@shellscape
Copy link
Contributor

Hey folks. This is a saved-form message, but rest assured we mean every word. The Rollup team is attempting to clean up the Issues backlog in the hopes that the active and still-needed, still-relevant issues bubble up to the surface. With that, we're closing issues that have been open for an eon or two, and have gone stale like pirate hard-tack without activity.

We really appreciate the folks have taken the time to open and comment on this issue. Please don't confuse this closure with us not caring or dismissing your issue, feature request, discussion, or report. The issue will still be here, just in a closed state. If the issue pertains to a bug, please re-test for the bug on the latest version of Rollup and if present, please tag @shellscape and request a re-open, and we'll be happy to oblige.

@guybedford
Copy link
Contributor

I think it would be worth keeping a tracking issue open here, since Wasm tree-shaking support in future is very much a thing that would simply need support to move forward from an interested contributor / team / company.

@aminya
Copy link

aminya commented Aug 2, 2020

It would be nice to have this. rollup-plugin-rust also needs this for the bundler mode of wasm-pack.
wasm-tool/rollup-plugin-rust#7

import * as wasm from './index_bg.wasm';

This is already available in the Webpack plugin.
https://www.npmjs.com/package/@wasm-tool/wasm-pack-plugin

@Pauan
Copy link
Contributor

Pauan commented Aug 3, 2020

To be clear, rollup-plugin-rust does not need esm-integration, it works perfectly fine without it.

But esm-integration would make things a bit more convenient for users, since they wouldn't need to load the .wasm file asynchronously.

Alternatively, if top level await was supported then it could be used instead of esm-integration (it would have the same effect).

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

9 participants