webpack / webpack

A bundler for javascript and friends. Packs many modules into a few bundled assets. Code Splitting allows for loading parts of the application on demand. Through "loaders", modules can be CommonJs, AMD, ES6 modules, CSS, Images, JSON, Coffeescript, LESS, ... and your custom stuff.
https://webpack.js.org
MIT License
64.55k stars 8.77k forks source link

Use WebAssembly.instantiateStreaming instead of WebAssembly.compileStreaming #6433

Closed linclark closed 6 years ago

linclark commented 6 years ago

Unfortunately there are performance issues with WebAssembly.compileStreaming on iOS. On iOS devices, there is a limited amount of faster memory. Because of this, the engine doesn't know which kind of memory to compile for until instantiate is called.

If I understand correctly, for the JSC engine compileStreaming will just do a memcopy of the .wasm. It's only when instantiate is called that the Memory object is created. Then the engine knows whether it will be using the fast memory, or just a normal malloc'd Memory that needs bounds checks. At this point, it can generate the code appropriately.

Because of this, we plan to recommend that all bundlers use instantiateStreaming instead. Does this cause any issues for webpack?

sokra commented 6 years ago

yes there are multiple issues with that:


Pre-knowledge:

webpack tries to treat WASM like ESM. We try to apply all rules/assumptions that apply to ESM to WASM too. We assume that in future the WASM JS API may be superseeded by a integration of WASM into the ESM module graph.

This means the imports section in the WASM file is resolved like import statements in ESM and the export section is treated like export in ESM.

WASM modules also have a start section, which is executed when the WASM is instantiated.

In the WASM JS API imports are passed via importsObject to the instantiate method.


The ESM spec specifies multiple phases. One phase is the ModuleEvaluation. In this phase all modules are evaulated in the well defined order. This phase is synchronous. All modules evaluate in the same "tick".

When WASM are in the module graph this means:

  1. the start section is executed in the same "tick"
  2. all dependencies of the WASM are executed in the same "tick"
  3. the ESM importing the WASM is executed in the same "tick"

This behavior is not possible when using the Promise-version of instantiate. A Promise always delays it's fulfillment into another "tick".

It's only possible when using a sync version of instantiate (WebAssembly.Instance).

Note: Technically there could be a WASM without start section and without dependencies. In this case this doesn't apply. But we can't assume this is always the case.


webpack wants to download and compile the WASM in parallel to downloading the JS code. Using instantiateStreaming won't allow this (when the WASM has dependencies), because instantiate requires passing a importsObject. Creating the importsObject requires evaluating all dependencies/imports of the WASM, so these dependencies need to be downloaded before starting to download the WASM.

When using compileStreaming + new WebAssembly.Instance parallel download and compile is possible, because compileStreaming doesn't require an importsObject. The importsObject can be created when WASM and JS download has finished.


I also want to quote the WebAssembly spec. It states that compilation happens in a background thread in compile and instantiation happens on the main thread.

It's not clearly stated but in my optinion JSC's behavior is not spec-comform.


Other notes:

WASM also lacks the ability to use live bindings of imported identifiers. Instead the importsObject is spec'ed to be copied. This could have weird problems with circular dependencies and WASM.

It would be great to support getters in the importsObject and the be able to get the exports before the start section is executed.

TheLarkInn commented 6 years ago

cc @jsbastien @flagxor @rossberg I wanted to surface this to you all in hopes maybe we can bring this up for the next WASM CG Meeting if discussions are involved. Primarily around the above concerns.

jfbastien commented 6 years ago

This is worth discussing at the next CG meeting. @TheLarkInn, would you mind adding to the agenda (once it's posted... we just had the meeting today so I haven't created the agenda yet)?

To answer a few points:

  1. I think treating WebAssembly like ECMAScript modules is the right thing to do. The JSC implementation uses exactly the same C++ code under the hood, so our behavior should match up very well. Same as ESM, export are handled when the JS object is created (but before execution returns), import is handled at "link" time, and start is called during "evaluate" (after tables and memories have been initialized). That's what I think WebAssembly should standardize as ESM integration.
  2. IIUC the main issue you're encountering is that you can't inject yourself between object creation and link, correct? ESM just do it by construction, so even circular imports work.
  3. I assume that having the import object as an extra indirection layer is a no-go (especially if you link wasm-wasm with i64)?

Interesting points on the spec. It's not final and I don't think this wording is accurate. It cannot mandate parallelism (unless parallel == 1 is OK). It does not mandate that all compilation be performed (disallowing any future compilation), and cannot either because that would prevent having an interpreter or tiering (so Chakra and SpiderMonkey would then not conform either).

kmiller68 commented 6 years ago

@sokra For what it's worth, the original design of Wasm actually differs from what is in the initial spec. See the pull request on the design: https://github.com/WebAssembly/design/pull/1018. I'm not sure where this got lost in the transition to the formal spec but I'll file an issue.

TheLarkInn commented 6 years ago

@kmiller68, so are you saying both methods can be used in paralell?

lukewagner commented 6 years ago

To discourage devs from using the sync WebAssembly.Instance and Module ctors (which, do sync compilation on various engines), Chrome currently throws when these constructors are called with modules bigger than an internal limit (4kb, last I saw). So that, combined with the fact that it's the worst-case load-time behavior for JSC, means sync new Instance should not be used by bundlers.

The ESM spec specifies multiple phases. One phase is the ModuleEvaluation. In this phase all modules are evaulated in the well defined order. This phase is synchronous. All modules evaluate in the same "tick".

I wasn't aware of this spec requirement, but you seem to be quite right!

So given these two constraints, could WebPack:

  1. rewrite the .wasm to remove the start section and add the start function as an export
  2. call WebAssembly.instantiateStreaming() during the instantiation phase
  3. call the previously-start function synchronously during the evaluation phase

To break most cycles involving functions, I would imagine instantiateStreaming being passed shim functions that call the real imported function via a mutable var that gets filled in later.

The main problems with this are:

  1. direct .wasm->.wasm imports (and avoiding serializing instantiateStreaming requrests)
  2. non-function (i.e., memory, table, global) wasm imports (for which the above shim-calling-a-var trick won't work)

While there are future JS API extensions we've discussed that could address some of these cases, I think it might be ok for a bundler to simply reject these cases (i.e., issue a build error) for now:

However, if these cases are or become important, I think they could be made to work, with some effort; happy to talk more about that.

sokra commented 6 years ago

rewrite the .wasm to remove the start section and add the start function as an export

I also thought about that, but that only works great when WASM has no dependencies.

shim functions direct .wasm->.wasm imports

Shim function would really defeat future performance improvements in JS engines. I think wasm-to-wasm calls will get optimized in engines soon. The benefit of not going through the JS stack is big (afaik at least for v8). (cc @mstarzinger)

non-function

This would break a lot of good stuff in WASM. No memory sharing, no tables...


In my opinion this is not a good way to go. I guess I'll stay with the sync Instance call for now. (Maybe using instantiateStreaming for WASM without start and only function imports from JS.) I hope browsers will allow sync Instance > 4kb. I mean they can still do off-thread compilation in compile and off-thread lazy optimization after instantiation... This works for JS why should it be a problem for WASM.

lukewagner commented 6 years ago

I also thought about that, but that only works great when WASM has no dependencies.

Are there any problems in addition to what is already being discussed?

Shim function would really defeat future performance improvements in JS engines. I think wasm-to-wasm calls will get optimized in engines soon. The benefit of not going through the JS stack is big (afaik at least for v8). (cc @mstarzinger)

When I mentioned shim functions, this was for JS↔wasm imports; and even then they weren't always necessary; just to break cycles. I gave some reasons above why I don't expect wasm↔wasm imports to be common at the bundler stage; have you seen any indications otherwise?

By the time wasm↔wasm imports have become common, we can have increased the expressiveness of JS API (in ways already proposed, e.g., allowing instantiate to accept Promises for imported values) or simply have native ESM-wasm integration.

non-function

This would break a lot of good stuff in WASM. No memory sharing, no tables...

Similarly, until dynamic linking starts being used, every module (or .wasm+.js glue pair) will have its own memory/table, so I don't think we need to worry about multiple .wasm's sharing the same memory/table in the initial support.

That being said, if you wanted to do extra work and support this, it would be possible with another simple .wasm rewrite: convert the module that defines the memory/table to instead import and then create the memory/table in the JS glue code so they can be imported.


I guess I'll stay with the sync Instance call for now. (Maybe using instantiateStreaming for WASM without start and only function imports from JS.) I hope browsers will allow sync Instance > 4kb.

Then in practice WebPack wasm integration won't work on Chrome and will have worst-case load-time performance on Safari; that seems pretty unattractive. I think it's important not to let the perfect be the enemy of the good here: we're talking about very simple configurations of .wasm modules for the first few years that could easily and optimally use today's instantiateStreaming.

jdalton commented 6 years ago

@lukewagner The 4 kB limit is a super bummer. Do you know if that applies to the API exposed in Node too?

lukewagner commented 6 years ago

Sorry, I don't

jdalton commented 6 years ago

For my use case I need the synchronous form for CJS to allow migrating to the asynchronous ESM form. But without the synchronous option I can't really offer the ESM form any time soon since there's no migration path.

The 4 kB limit seems odd since the file is already downloaded, so not blocking for network. It's just CPU crunching which could be done in something like a web worker.

Update:

Grabbing some larger wasm payloads, 15 kB and 41 kB, I was able to get output from:

WebAssembly.Module.exports(new WebAssembly.Module(fs.readFileSync("./large.wasm")))

Which I'm guessing means there is no restriction there (a good thing).

rossberg commented 6 years ago

@jdalton, a web worker only helps if it can run concurrently, which necessitates an asynchronous API.

Still, not everybody is happy with the 4kB limit for synchronous compilation. Web platform folks felt very strongly about it, though. I don't actually know if V8 activates it on Node as well.

jdalton commented 6 years ago

A web worker can run the synchronous Module call off the UI thread. I think the 4 kB limit is a step too far. We get it, synchronous isn't ideal, but it has a place. Putting a blocker in it like this is stop-energy that hinders adoption and worse fosters bad feels among those folks with the ecosystem impact to encourage or discourage further adoption.

rossberg commented 6 years ago

What's the point of off-loading to another thread when the UI thread has to block on that other thread finishing?

Agreed with the rest.

jdalton commented 6 years ago

What's the point of off-loading to another thread when the UI thread has to block on that other thread finishing?

I'm not going to bikeshed various web worker implementations. You get the gist, it's gravy when you can offload to another thread for crunching.

Agreed with the rest.

🙌

lukewagner commented 6 years ago

@jdalton Just confirmed also; I guess it's a browser-only limitation.

sokra commented 6 years ago

When I mentioned shim functions, this was for JS↔wasm imports; and even then they weren't always necessary; just to break cycles.

cycles are not the problem in this case (they are another problem).

Consider a part of you application is loaded on demand and contains WASM (this is always the case with webpack when using WASM). Assuming the WASM module depends on some other JS module in that part of the application.

When the user requests that part of the application I want to start downloading the JS and the WASM. I also want to prepare the WASM to be used. This could happen in parallel.

Now the problem with instanciateStreaming. You need to pass all imports to instanciateStreaming. But these imports do come from the JS module which should be loaded in parallel. I could solve this with shim functions, but I always need them.

Using compileStreaming + sync Instance make parallel download and compilation possible.

I gave some reasons above why I don't expect wasm↔wasm imports to be common at the bundler stage; have you seen any indications otherwise?

I my opinion this it not uncommon. I imagine the WASM future as many small WASM modules interleaved with JS modules. WASM to WASM is possible and great, because you could pass complex binary objects between them instead of converting to JS values. Many WASM modules can share a memory WASM module (which gives you alloc and free) and pass pointer around. WASM-to-WASM is great and we shouldn't limit or prevent it.

as long as dynamic linking isn't well supported

Everything for dynamic linking with imports and exports are in place. Any performance issues left with this could be easily solved. In my option this is the way to go.

I favor dynamic linking over static linking because:

Dynamic linking should be the way you author WASM.

Are there any problems in addition to what is already being discussed?

You may use WASM from CommonJS, conditionally:

if(Math.random() < 0.5)
  require("./something.wasm");

This must execute sync. In this case webpack calls compileStreaming while chunk load and new Instance when the module is actually used.

Are there any problems in addition to what is already being discussed?


My conclusion:

The ESM spec requires sync evaluation, that's only possible with new WASM.Instance, while keeping the full WASM feature set.

Either the ESM spec is changed to allow async evaluation, which is don't recommend.

Or the WASM implementations are changed to allow and support sync instantiate. The WASM spec doesn't have to be changed, it already allows sync instantiate.

Or we give up on the approach to make WASM like ESM.

rossberg commented 6 years ago

@sokra:

Now the problem with instanciateStreaming. You need to pass all imports to instanciateStreaming. But these imports do come from the JS module which should be loaded in parallel. I could solve this with shim functions, but I always need them.

We have discussed generalising the instantiate function to allow its import object to be a collection of promises of individual objects (or even allow the individual entities to be promises). That should solve the parallel compilation use case without requiring shims.

lukewagner commented 6 years ago

Ah, ok, I was imagining the JS file that called instantiateStreaming also contained all the JS code (it had already been downloaded), but it sounds like these are in separate files, so I see why function shims would always be needed.

Now the problem with instanciateStreaming. You need to pass all imports to instanciateStreaming. But these imports do come from the JS module which should be loaded in parallel. I could solve this > with shim functions, but I always need them.

Agreed with @rossberg here, using shims as a polyfill.

I my opinion this it not uncommon. I imagine the WASM future as many small WASM modules interleaved with JS modules. WASM to WASM is possible and great, because you could pass complex binary objects between them instead of converting to JS values. Many WASM modules can share a memory WASM module (which gives you alloc and free) and pass pointer around.

It's not possible to take two arbitrary wasm modules and have them share memory and tables; they have to be specifically compiled (probably with the same toolchain for the same language) to work this way. This is because these wasm modules must carefully coordinate use of linear memory and table index space to avoid clobbering each other and also coordinate and basic impl details like malloc/free. Right now, there is not a non-experimental toolchain that does this that I know of.

Moreover, even when this is possible, I don't think it will be common for multiple wasm ESMs to share memory because it is, I think, a quite good thing for unrelated modules to have separate memories/tables: otherwise there will be subtle corruption bugs that only surface when particular modules are used together, leaks will be hard to track down, and OOMs on 32-bit devices will be more likely due to larger memories. It will probably be a good idea in the future to use dynamic linking to factor out common runtime code (e.g., libc, libm), but in that case it is only the Module being shared between Instances, not the Memory or Table.

Everything for dynamic linking with imports and exports are in place.

Not in the toolchain: it may be possible at the bundler level but, right now, there is not a non-experimental toolchain that will actually emit dynamically-linked wasm code.


Why can't we go with the shims? I think they'll work fine w/ very little overhead (probably zero overhead once the Promise-taking instantiateStreaming is present).

TheLarkInn commented 6 years ago

Ignoring multi lang modules compiled to wasm, I don't see why a narrowerer scope of single lang compiled WASM modules sharing memory being a bad idea.

On Mon, Feb 19, 2018, 1:16 PM Luke Wagner notifications@github.com wrote:

Ah, ok, I was imagining the JS file that called instantiateStreaming also contained all the JS code (it had already been downloaded), but it sounds like these are in separate files, so I see why function shims would always be needed.

Now the problem with instanciateStreaming. You need to pass all imports to instanciateStreaming. But these imports do come from the JS module which should be loaded in parallel. I could solve this > with shim functions, but I always need them.

Agreed with @rossberg https://github.com/rossberg here, using shims as a polyfill.

I my opinion this it not uncommon. I imagine the WASM future as many small WASM modules interleaved with JS modules. WASM to WASM is possible and great, because you could pass complex binary objects between them instead of converting to JS values. Many WASM modules can share a memory WASM module (which gives you alloc and free) and pass pointer around.

It's not possible to take two arbitrary wasm modules and have them share memory and tables; they have to be specifically compiled (probably with the same toolchain for the same language) to work this way. This is because these wasm modules must carefully coordinate use of linear memory and table index space to avoid clobbering each other and also coordinate and basic impl details like malloc/free. Right now, there is not a non-experimental toolchain that does this that I know of.

Moreover, even when this is possible, I don't think it will be common for multiple wasm ESMs to share memory because it is, I think, a quite good thing for unrelated modules to have separate memories/tables: otherwise there will be subtle corruption bugs that only surface when particular modules are used together, leaks will be hard to track down, and OOMs on 32-bit devices will be more likely due to larger memories. It will probably be a good idea in the future to use dynamic linking to factor out common runtime code (e.g., libc, libm), but in that case it is only the Module being shared between Instances, not the Memory or Table.

Everything for dynamic linking with imports and exports are in place.

Not in the toolchain: it may be possible at the bundler level but, right now, there is not a non-experimental toolchain that will actually emit dynamically-linked wasm code.

Why can't we go with the shims? I think they'll work fine w/ very little overhead (probably zero overhead once the Promise-taking instantiateStreaming is present).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/webpack/webpack/issues/6433#issuecomment-366804432, or mute the thread https://github.com/notifications/unsubscribe-auth/ADQBMPV7ecdBl1w7wld8zOKPdiazKgIqks5tWeSXgaJpZM4R2Qni .

lukewagner commented 6 years ago

Practically speaking, dynamic linking of unrelated modules (i.e., modules that aren't compiled all together as part of the same program/app/system) requires a stable source-language ABI which doesn't exist for any of compile-to-wasm language I've seen (and probably shouldn't until ABI-affecting wasm features like multiple return values and exception handling land). So I think that's the entire story for the next 1-2 years unless something totally new pops up.

But even after that, I think modules should default to being standalone and defining their own memory. The benefits are basically the same as those for having a microkernel OS architecture:

We can definitely hypothesize situations where it would be more efficient for modules to share memory, but there are a couple different ways to go about fixing that and it's not clear that a brute-force type of dynamic linking is the right way. It's be useful to have the use cases present to help guide us through this design space since it's inherently in optimization territory.

sokra commented 6 years ago

Ok I created an extra challenge.

Here is a test case which I believe is impossible to solve without sync Instance.

https://github.com/webpack/webpack/pull/6529/files

The dependency graph looks like this:

a -> tracker
  -> b -> tracker
  -> wasm -> tracker
          -> c

Which results in the following evaluation order according to the ESM: tracker, b, c, wasm, a (all in the same tick)

Feel free to "compile" the test case anyway you like and try using instanciateStreaming. Even when Promises as imports would be allowed, it won't work in my opinion.

But maybe you surprise me with a clever approach and I was wrong.

lukewagner commented 6 years ago

I may be missing the new challenge that's in your example, but why do the steps 1-3 that I mentioned above not work (admittedly, using shims)? In particular, by the time the ModuleEvaluation tick started, wasm's instantiateStreaming would have already resolved (during the instantiate phase) and its formerly-start function could be called synchronously at the right time.

TheLarkInn commented 6 years ago

@lukewagner I personally would like to see shims as a last-ditch effort.

Beyond "general discouraging for developers using the Sync cases", are there underlying issues that browsers are having with implementing or supporting this that are not mentioned? It seems like there is some sort of underlying concern here that hasn't surfaced itself?

We are hoping to have support for out of the box via our implementation:

Which I would assume raises concerns for steps 1-3.

lukewagner commented 6 years ago

Some engines (the set of which may change over time) need to do a compilation/specialization step during instantiation based on the physical characteristics of the received Memory (see Section 7.1 for more details). Using WebAssembly.instantiateStreaming ensures no synchronous janks if this happens.

I assume the shims we're talking here are of the form:

var someImport_;  // initialized later in the script
function someImport(a,b,c) { return someImport_(a,b,c); }
WebAssembly.instantiateStreaming({someModule:{someImport}});

This seems like pretty modest overhead, maybe not even measurable for realistic uses (engines aggressively inline). I don't see why this would be considered a last-ditch effort, especially if the shim can go away with the Promise-taking APIs.

lukewagner commented 6 years ago

Oops, I meant to also respond to:

non-function (i.e., memory, table, global) wasm imports (for which the above shim-calling-a-var trick won't work)

For this, what I suggested above was that, when a.wasm imports a table/memory/global defined and exported by wasm module b.wasm (perhaps transitively), b.wasm can be rewritten to also import the table/memory/global and then the table/memory/global can be created via the JS API and imported by both. Could this work? It seems like all of this wiring up happens purely during the ESM instantiation phase.

sokra commented 6 years ago

Some engines (the set of which may change over time) need to do a compilation/specialization step during instantiation based on the physical characteristics of the received Memory (see Section 7.1 for more details). Using WebAssembly.instantiateStreaming ensures no synchronous janks if this happens.

They could compile the code during compilation phase and only do the memory base patching while instantation. Memory base patching must be fast and sync anyway because it is executed on grow and shrink memory too.

I may be missing the new challenge that's in your example, but why do the steps 1-3 that I mentioned above not work (admittedly, using shims)? In particular, by the time the ModuleEvaluation tick started, wasm's instantiateStreaming would have already resolved (during the instantiate phase) and its formerly-start function could be called synchronously at the right time.

Shims doesn't work for the magicNumber imported from c. You could argue that get_global could be rewritten to a imported function call (shim), but that would only work for primitive types and not for memory or tables.

For this, what I suggested above was that, when a.wasm imports a table/memory/global defined and exported by wasm module b.wasm (perhaps transitively), b.wasm can be rewritten to also import the table/memory/global and then the table/memory/global can be created via the JS API and imported by both. Could this work? It seems like all of this wiring up happens purely during the ESM instantiation phase.

This would really work for memory and tables.


Ok I guess we are now at a point at which all these tricks/hacks and wasm rewriting can combine to something that would allow using instantiateStreaming with only a small limitation: memory and tables can't be imported from JS modules.

I'll summarize it in my next comment.

lukewagner commented 6 years ago

Thanks a lot for all the consideration!

They could compile the code during compilation phase and only do the memory base patching while instantation. Memory base patching must be fast and sync anyway because it is executed on grow and shrink memory too.

The memory patching is not the issue, but rather the reservation of ~8gb memories; OS quota limitations can prevent always grabbing this much virtual memory leading to completely different codegen being necessary.

with only a small limitation: memory and tables can't be imported from JS modules.

If the memory/table definitions were hoisted out of wasm modules and into new WebAssembly.Memory/Table JS API calls up front, couldn't these be imported by JS modules just as well as wasm modules?

sokra commented 6 years ago

The following rewriting happens to the WASM:

The following happens on chunk load:

We create a prepared wasm object (We name this object A in the following).

We create a imports object:

For X1 we create a new WebAssembly.Memory with initial size extracted from WASM. We assign this memory instance as property to A. For X2 we create a new WebAssembly.Table. We assign the instance as property to A.

For an imported memory or table from another WASM we can get the value from A of the other module. We need to make sure that these are called in correct order.

For an imported global we create a function which returns __webpack_require__.c[moduleId].exports[exportName]. We will make sure that the module is loaded before wasm code is used.

For an imported function we create a function function(a, b, c) { return __webpack_require__.c[moduleId].exports[exportName](a, b, c); }.

If Promises in the importsObject would be possible we could pass a Promise for imported functions from other WASM.

We call instantiateStreaming with the imports object.

On Promise completion we store the exports object and the start function to A. On Promise rejection we store the error in A.

We also need to add imports from X1 and X2 to the exports object.

The following happen on require call:

We throw if an error is stored in A.

We assign the exports object from A to the real exports object.

We call the start function.

Limitations

Benefits

sokra commented 6 years ago

If the memory/table definitions were hoisted out of wasm modules and into new WebAssembly.Memory/Table JS API calls up front, couldn't these be imported by JS modules just as well as wasm modules?

I was talking about the case when ESM exports a memory object and WASM imports it.

export default new WebAssembly.Memory(..) and (memory (import "./x" "default"))

Is this required for threading in WASM? SharedArrayBuffer?

flagxor commented 6 years ago

Catching up on this thread late.

Few things on the 4KB limit:

I suspect we should start looking into Promisified instantiateStreaming, as it seems the need has popped up.

jdalton commented 6 years ago

Few things on the 4KB limit:

Providing a partially broken experience isn't great. Could it be done with a console.warn to inform devs, instead of a hard fail for those situations? Though the roundabout API itself is already a kind of opt-in to "Yes, I understand the risks".

lukewagner commented 6 years ago

Sounds great!

rewrite import global to import function returning the global type

Arg, right. As soon as mutable globals are available, they could be used instead of function calls. One tricky case is that constant global imports can be used within initializer expressions (to position data/elem segments). However, that functionality is only needed for dynamic linking (when a module doesn't know where it lives in linear memory), so probably it's fine for this not to be supported yet. The bulk memory operations proposal would allow data/elem segment initialization at runtime which would in turn allow these uses of constant global import initializer expressions to be rewritten into something that would execute during the ModuleEvaluation phase.

I was talking about the case when ESM exports a memory object and WASM imports it.

Ah hah, I see what you mean. IIUC, this wouldn't work even with native ESM-wasm integration, assuming that wasm instantiation occurs during the overall ESM instantiation phase (since new WebAssembly.Memory(...) is a dynamic expression that needs to be evaluated during the ModuleEvaluation phase). For this to work, it seems like JS would need some sort of syntactic way to declaratively export memory...

Practically speaking, I think this means that, for the purposes of sharing Memory between wasm modules, you'd want some small memory.wasm that defines and exports the Memory, making it available during the instantiation phase. This might actually make a lot of sense, since that memory.wasm could also own the memory bookkeeping and export mmap/munmap functions to all dependent wasm modules so they don't all clobber each other.

flagxor commented 6 years ago

Raising new Instance higher than new Module might make sense.

Though we'd need to be thoughtful about leaving latitude for different implementation strategies (like compile at instantiate). Lowering the limit once it's been raised would be hard. It seems like for modules, being able to use instantiateStreaming is ideal.

Could it be done with a console.warn to inform devs, instead of a hard fail for those situations?

In general, I've been encouraged to have less chatty things on the console by devtools folks. Not sure this is always ideal. I often wish we had similarly verbose compilation and caching info to Firefox for asm.js/wasm. Though for this one it might make sense.

xtuc commented 6 years ago

I'm working on a PR to fix this and i'm trying to find the best solution.

The one describe here forces us to shift:

since import i32 global ⟶ export func that returns the i32 global, which is not ideal.


My idea is to inject a runtime func that Webpack will call first (and eventually call the start func).

For example:

(module
  (import "env" "global" (global i32))
  (func (export "t") (result i32)
    (get_global 0)
  )
)

Transformations:

(module
  (global (mut i32) (i32.const 0))
  (func (export "t") (result i32)
    (get_global 0)
  )
  (func (export "init") (param i32)
    (get_local 0)
    (set_global 0)
  )
)

Each global will add an argument and two instructions, according to v8's limits that should cover most use-cases, but if not we can inject another init function.

Since you are the experts here, I would like to know what do you think about that? And if i'm not missing obvious things.

It's also trivial to import an i64 global when standardized even if the host doesn't support it.

lukewagner commented 6 years ago

Thanks for asking! Talking with @linclark and @rossberg a bit more over the last week about how native wasm-ESM integration would be defined in terms of core wasm semantics, I think the natural thing for wasm would be to have wasm instantiation take a snapshot of the values of its imported live bindings at the point in the Instantiation-phase post-order traversal where the wasm module gets instantiated. This reflects the existing design of the wasm core spec's embedding interface, which takes a list of values. (WebAssembly module exports could still be live bindings which would allow a degree of cyclicy with JS.)

Unfortunately, IIUC, the only live bindings with non-undefined values during the Instantiation phase would be (1) JS functions exported by JS, (2) wasm definitions (= memories, tables, globals, functions) exported by wasm. (In particular, even if you have export var foo = 42, the 42 is not stored into foo's binding until the Evaluate phase.)

With the mutable globals and an any reference type, we could perhaps define any random JS export to be importable with type (global mut any) which would allow the imported value to be read "live" by get_global during the Evaluate phase, just like a normal ESM imports. But that's farther in the future.

But for now, this means that global imports (constant and mutable) are in the same category as memory/table imports. Note: global imports exist in wasm v.1 solely for dynamic linking (specifically, to pipe in a dynamic value for the offset of a data/elem segment) which, as I was explaining above, probably won't be used for a while.

So my (tentative, obviously this is all in flux and open for discussion :) recommendation would be, for an MVP, to simply not support bundling memory/table/global imports, or to support it by serializing instantiateStreaming calls. Once the abovementioned Promises-in-import-objects extension gets standardized and starts shipping, it could be used to implement the above "snapshot" instantiation semantics with full fidelity and at no runtime cost.

xtuc commented 6 years ago

I was thinking about the implementation specifically for Webpack but it would be more useful to integrate it in the standard, I agree.

Since in Webpack we can resolve the dependencies AOT, we can optimize the loading of the modules (like instantiateStreaming'ing multiple modules at the same time) and just linking them together once someone access it (basically my PR).

Once the abovementioned Promises-in-import-objects extension gets standardized

I like that solution, it would simplify what we're trying to do here.

I'm just wondering, if you have a cyclic dependency it will end up in a deadlock (just the concept nothing to do with lock etc)? It seems less flexible that functions.


To sum up and from what I understand we should leave the compileStreaming as is and wait for standard solutions, right?

One idea behind the init function from my PR is that we can already think about polyfilling future features (like the i64 global import).

lukewagner commented 6 years ago

I'm just wondering, if you have a cyclic dependency it will end up in a deadlock (just the concept nothing to do with lock etc)?

It would lead to an instantiation-time error when one wasm module's instantiation would read undefined from the binding, if that's what you mean.

To sum up and from what I understand we should leave the compileStreaming as is and wait for standard solutions, right?

In the short term, it would still be best to switch to instantiateStreaming, for reasons described above; This shouldn't block on the Promise-in-import extension

mathiasbynens commented 6 years ago

FWIW, using WebAssembly.instantiateStreaming where applicable matches Google’s official recommendation on how to load WebAssembly modules efficiently.

jdalton commented 6 years ago

@mathiasbynens I think that's great for general consumption, however I believe the root issue here is that webpack itself is using synchronous APIs in a more responsible manner (in workers?) and is only being blocked by an arbitrary 4kB limit imposed by Chrome browser specifically. (It's been a while so things might have shifted)

Related: https://github.com/webpack/webpack/issues/6475#issuecomment-381161119.

jfbastien commented 6 years ago

@jdalton AFAIK Chrome's 4kB limits on sync compilation should only be in effect for non-main-thread compilation. If that's not the case then it's a bug in Chrome.

gahaas commented 6 years ago

@jdalton Synchronous compilation should work in Chrome 66 on workers. There was indeed an issue but it got fixed.

TheLarkInn commented 6 years ago

Interesting... @sokra spinning into a worker would mix a few features that we can/want to land.

On Fri, Apr 13, 2018, 9:27 AM gahaas notifications@github.com wrote:

Synchronous compilation should work in Chrome 66 on workers. There was indeed an issue but it got fixed.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/webpack/webpack/issues/6433#issuecomment-381189335, or mute the thread https://github.com/notifications/unsubscribe-auth/ADQBMEQoz00APeilDbQy5CX4WebROt5Tks5toNH1gaJpZM4R2Qni .

gahaas commented 6 years ago

@jdalton I mixed up issues. For me, already in Chrome 65 new WebAssembly.Module works in workers without size limit on the input. If you can send me an example where it does not work, I will investigate that issue.

FYI, I mixed it up with WebAssembly.compiledStreaming, which is only supported in Chrome 66 on workers.

mathiasbynens commented 6 years ago

@jfbastien

AFAIK Chrome's 4kB limits on sync compilation should only be in effect for non-main-thread compilation.

The opposite is true: the 4 KB size limits only apply on the main thread. There are no limits in workers. https://cs.chromium.org/chromium/src/third_party/blink/renderer/bindings/core/v8/v8_initializer.cc?l=437 (I guess you meant s/non-//?)

jfbastien commented 6 years ago

Yes.

sokra commented 6 years ago

This is fixed. We now use instantiateStreaming when possible. Falling back to compileStreaming + async instantiate for WASM-to-WASM dependency chains.

Falling back non-streaming variants when streaming is not supported by the browser.

sokra commented 6 years ago

Note: adding Promise support to the importsObject would allow us to remove the compileStreaming + async instantiate fallback.

cars10 commented 6 years ago

Can someone post an example about how to correctly import wasm now?

A simple

import * as wasm from 'hello_world.gc.wasm'

does not seem to work.

xtuc commented 6 years ago

@cars10 do you have an error?