-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Fix wasm loading in workers, and refactor async promise code #5011
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
Changes from all commits
ab7c291
82bd2c6
d1a46e4
9030c2b
546ec62
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2153,16 +2153,28 @@ function integrateWasmJS(Module) { | |
|
||
function getBinary() { | ||
var binary; | ||
if (ENVIRONMENT_IS_WEB || ENVIRONMENT_IS_WORKER) { | ||
if (Module['wasmBinary']) { | ||
binary = Module['wasmBinary']; | ||
assert(binary, "on the web, we need the wasm binary to be preloaded and set on Module['wasmBinary']. emcc.py will do that for you when generating HTML (but not JS)"); | ||
binary = new Uint8Array(binary); | ||
} else { | ||
} else if (Module['readBinary']) { | ||
binary = Module['readBinary'](wasmBinaryFile); | ||
} else { | ||
throw "on the web, we need the wasm binary to be preloaded and set on Module['wasmBinary']. emcc.py will do that for you when generating HTML (but not JS)"; | ||
} | ||
return binary; | ||
} | ||
|
||
function getBinaryPromise() { | ||
// if we don't have the binary yet, and have the Fetch api, use that | ||
if (!Module['wasmBinary'] && typeof fetch === 'function') { | ||
return fetch(wasmBinaryFile).then(function(response) { return response.arrayBuffer() }); | ||
} | ||
// Otherwise, getBinary should be able to get it synchronously | ||
return new Promise(function(resolve, reject) { | ||
resolve(getBinary()); | ||
}); | ||
} | ||
|
||
// do-method functions | ||
|
||
function doJustAsm(global, env, providedBuffer) { | ||
|
@@ -2212,7 +2224,9 @@ function integrateWasmJS(Module) { | |
#if BINARYEN_ASYNC_COMPILATION | ||
Module['printErr']('asynchronously preparing wasm'); | ||
addRunDependency('wasm-instantiate'); // we can't run yet | ||
WebAssembly.instantiate(getBinary(), info).then(function(output) { | ||
getBinaryPromise().then(function(binary) { | ||
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The main .html file is supposed to download the .wasm file. At this point, is it not possible that the .wasm file download is still in progress, but has not finished yet, and therefore There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not as it currently works - the html downloads the wasm binary before starting to do anything else. Another reason it's good to get rid of that thing ;) |
||
return WebAssembly.instantiate(binary, info) | ||
}).then(function(output) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey, since working on this, can you change the #if #endif in here to a #if #else #endif to avoid the "unreachable code" console warnings below? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea, fixing. |
||
// receiveInstance() will swap in the exports (to Module.asm) so they can be called | ||
receiveInstance(output.instance); | ||
removeRunDependency('wasm-instantiate'); | ||
|
@@ -2221,7 +2235,7 @@ function integrateWasmJS(Module) { | |
Module['quit'](1, reason); | ||
}); | ||
return {}; // no exports yet; we'll fill them in later | ||
#endif | ||
#else | ||
var instance; | ||
try { | ||
instance = new WebAssembly.Instance(new WebAssembly.Module(getBinary()), info) | ||
|
@@ -2234,6 +2248,7 @@ function integrateWasmJS(Module) { | |
} | ||
receiveInstance(instance); | ||
return exports; | ||
#endif | ||
} | ||
|
||
function doWasmPolyfill(global, env, providedBuffer, method) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need this getBinary() function as well, or could we just have one getBinaryPromise() function that also contains the content of this function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use it when we need synchronous access to it - for the interpreter, or when async compilation is off. Also, I think it's nice it's separate, conceptually.