Use WebAssembly.compileStreaming API if available

[dschuff]

This API is a post-MVP addition
(http://webassembly.org/docs/web/#additional-web-embedding-api) that does
streaming compilation (i.e compiles the wasm binary as it downloads), resulting
in faster instantiation in the non-cached case.

[dschuff]

Any good ideas how to test this? It requires the server to use the proper MIME type for the binary, so to test at all we'd have to have the test server do that; and to test the fallback case, we'd have to make that configurable. In any case if it's reasonable we might want to make the test server use this, since it will eventually hopefully be the most common path.

[nazar-pc]

Shouldn't this line be in else branch?

[dschuff]

The previous if ends with a return, so no need.

[nazar-pc]

I'd prefer else here, since there is one more return {}; one line below, now it is duplicated in 2 places.

[jgravelle-google]

Additional nit: if we invert the conditional to put the shorter branch first, that's probably more readable whether we early-return or if-else.
Not sure if the condition is more readable as !Module['wasmBinary'] && typeof WebAssembly.instantiateStreaming === 'function' or Module['wasmBinary'] || typeof WebAssembly.instantiateStreaming !== 'function', probably the former? So, var canStream = ...; if (!canStream) { ...

[dschuff]

I'm a little torn on that because I usually prefer positive conditions ("we can stream") and to put the "expected" case first and the unusual/exceptional case second. I went ahead and put the else in since there's only 2 cases and to deduplicate the return. How does it look now?

[dschuff]

In any case based on the discussion in #5577 we may end up with a different condition anyway...

[jgravelle-google]

The benefits of shorter conditionals first are more notable when composed with other if/elses, if (!foo) { bar(); return; } baz(); combines with itself much more nicely than if (foo) { baz(); } else { bar(); } does (see also, the arrow anti-pattern).
Here the function callbacks scopes are sort of serving as additional pushes and pops on the reader's mental stack. Currently the else is only ten lines away from the if so it isn't too bad, so I'm not too up in arms about it here.

[dschuff]

I feel like that's an argument for early-return, more so than for shorter conditionals. I think in practice that does often mean short error cases go first and get early-returns, like in your linked example. But that's still "if (positive case) then error" and I want "if (positive case) then stream". I do think that if we had more than just 2 cases then the order would probably be determined by how they composed and/or how readable each condition was. With just 2 fairly-short cases it's less obvious. And also the stakes are much lower, so 🤷‍♀️

[kripken]

The indentation made this a little hard for me to read (to see where the .s go etc.). How about

WebAssembly.instantiateStreaming(
  fetch(wasmBinaryFile, { credentials: 'same-origin' }), info)
    .then(receiveInstantiatedSource)
    .catch(function(reason) {
      // We expect the most common failure cause to be a bad MIME type for the binary,
      // in which case falling back to ArrayBuffer instantiation should work.
      Module['printErr']('wasm streaming compile failed: ' + reason);
      Module['printErr']('falling back to ArrayBuffer instantiation');
      instantiateArrayBuffer(receiveInstantiatedSource);
    })
);

Also I had to add a ) so I think one might have been missing before.

[dschuff]

Actually, that indentation isn't right because the .then and .catch are not attached to the promise returned by fetch, they go with the return of instantiateStreaming. The fetch's promise is the first argument to instantiateStreaming. (That is also the reason why the number of parens is correct). I did make the .catch line up with the .then though, agreed that's better.

[kripken]

cool, looks nice.

[dschuff]

I pushed a change to use the correct MIME type in the test server. can someone remind me how to run the browser tests with wasm turned on?

[kripken]

browser.test_binaryen* will run the wasm-specific tests. To run the other tests in wasm mode, one way is to edit src/settings.js and set BINARYEN to 1.

[dschuff]

OK, I verified that the test runner update does what I want (namely, set the MIME type correctly for wasm files, which makes instantiateStreaming work). So I think this patch is ready to go in. WRT use of fetch, we can continue the discussion on the other issue. This change won't break any current Electron apps because Electron's version of Chromium doesn't have instantiateStreaming yet anyway.

[kripken]

lgtm assuming the browser.test_binaryen* tests still pass.