-
Notifications
You must be signed in to change notification settings - Fork 80
JSPI and Asyncify #551
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
base: main
Are you sure you want to change the base?
JSPI and Asyncify #551
Changes from all commits
e8de9d4
5fe83eb
9a2014c
2bb83fd
4027d11
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 |
---|---|---|
|
@@ -5,6 +5,10 @@ use std::fmt::Write; | |
|
||
#[derive(Copy, Clone, Ord, PartialOrd, Eq, PartialEq)] | ||
pub enum Intrinsic { | ||
AsyncifyAsyncInstantiate, | ||
AsyncifySyncInstantiate, | ||
AsyncifyWrapExport, | ||
AsyncifyWrapImport, | ||
Base64Compile, | ||
ClampGuest, | ||
ComponentError, | ||
|
@@ -23,6 +27,7 @@ pub enum Intrinsic { | |
HasOwnProperty, | ||
I32ToF32, | ||
I64ToF64, | ||
Imports, | ||
InstantiateCore, | ||
IsLE, | ||
ResourceTableFlag, | ||
|
@@ -114,6 +119,117 @@ pub fn render_intrinsics( | |
|
||
for i in intrinsics.iter() { | ||
match i { | ||
Intrinsic::AsyncifyAsyncInstantiate => output.push_str(" | ||
const asyncifyModules = []; | ||
let asyncifyPromise; | ||
let asyncifyResolved; | ||
async function asyncifyInstantiate(module, imports) { | ||
const instance = await instantiateCore(module, imports); | ||
const memory = instance.exports.memory || (imports && imports.env && imports.env.memory); | ||
const realloc = instance.exports.cabi_realloc || instance.exports.cabi_export_realloc; | ||
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. Same as for the memory, this should be an argument. |
||
if (instance.exports.asyncify_get_state && memory) { | ||
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. What if we don't have a memory here or asyncify_get_state? Does the fallthrough work, or should we throw an error? |
||
let address; | ||
if (realloc) { | ||
address = realloc(0, 0, 4, 1024); | ||
new Int32Array(memory.buffer, address).set([address + 8, address + 1024]); | ||
} else { | ||
address = 16; | ||
new Int32Array(memory.buffer, address).set([address + 8, address + 1024]); | ||
} | ||
asyncifyModules.push({ instance, memory, address }); | ||
} | ||
return instance; | ||
} | ||
function asyncifyState() { | ||
return asyncifyModules[0]?.instance.exports.asyncify_get_state(); | ||
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. Should this function take an asyncify module index? |
||
} | ||
function asyncifyAssertNoneState() { | ||
let state = asyncifyState(); | ||
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. Should this assert across all asyncify module indices? |
||
if (state !== 0) { | ||
throw new Error(`reentrancy not supported, expected asyncify state '0' but found '${state}'`); | ||
} | ||
} | ||
"), | ||
|
||
Intrinsic::AsyncifySyncInstantiate => output.push_str(" | ||
const asyncifyModules = []; | ||
let asyncifyPromise; | ||
let asyncifyResolved; | ||
function asyncifyInstantiate(module, imports) { | ||
const instance = instantiateCore(module, imports); | ||
const memory = instance.exports.memory || (imports && imports.env && imports.env.memory); | ||
const realloc = instance.exports.cabi_realloc || instance.exports.cabi_export_realloc; | ||
if (instance.exports.asyncify_get_state && memory) { | ||
let address; | ||
if (realloc) { | ||
address = realloc(0, 0, 4, 1024); | ||
new Int32Array(memory.buffer, address).set([address + 8, address + 1024]); | ||
} else { | ||
address = 16; | ||
new Int32Array(memory.buffer, address).set([address + 8, address + 1024]); | ||
} | ||
asyncifyModules.push({ instance, memory, address }); | ||
} | ||
return instance; | ||
} | ||
function asyncifyState() { | ||
return asyncifyModules[0]?.instance.exports.asyncify_get_state(); | ||
} | ||
function asyncifyAssertNoneState() { | ||
let state = asyncifyState(); | ||
if (state !== 0) { | ||
throw new Error(`reentrancy not supported, expected asyncify state '0' but found '${state}'`); | ||
} | ||
} | ||
"), | ||
|
||
Intrinsic::AsyncifyWrapExport => output.push_str(" | ||
function asyncifyWrapExport(fn) { | ||
return async (...args) => { | ||
if (asyncifyModules.length === 0) { | ||
throw new Error(`none of the Wasm modules were processed with wasm-opt asyncify`); | ||
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. If we didn't have the fall-through path on the asyncify instantiate, would we be able to turn this into a static instead of a runtime error? |
||
} | ||
asyncifyAssertNoneState(); | ||
let result = fn(...args); | ||
while (asyncifyState() === 1) { | ||
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. It sounds like the logic here is that the first module to be instantiated is the top of the call graph. What happens if the first module we instantiate is not the top of the asyncify call graph? |
||
asyncifyModules.forEach(({ instance }) => { | ||
instance.exports.asyncify_stop_unwind(); | ||
}); | ||
asyncifyResolved = await asyncifyPromise; | ||
asyncifyPromise = undefined; | ||
asyncifyAssertNoneState(); | ||
asyncifyModules.forEach(({ instance, address }) => { | ||
instance.exports.asyncify_start_rewind(address); | ||
}); | ||
result = fn(...args); | ||
} | ||
asyncifyAssertNoneState(); | ||
return result; | ||
}; | ||
} | ||
"), | ||
|
||
Intrinsic::AsyncifyWrapImport => output.push_str(" | ||
function asyncifyWrapImport(fn) { | ||
return (...args) => { | ||
if (asyncifyState() === 2) { | ||
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. Could this be something like |
||
asyncifyModules.forEach(({ instance }) => { | ||
instance.exports.asyncify_stop_rewind(); | ||
}); | ||
const ret = asyncifyResolved; | ||
asyncifyResolved = undefined; | ||
return ret; | ||
} | ||
asyncifyAssertNoneState(); | ||
let value = fn(...args); | ||
asyncifyModules.forEach(({ instance, address }) => { | ||
instance.exports.asyncify_start_unwind(address); | ||
}); | ||
asyncifyPromise = value; | ||
}; | ||
} | ||
"), | ||
|
||
Intrinsic::Base64Compile => if !no_nodejs_compat { | ||
output.push_str(" | ||
const base64Compile = str => WebAssembly.compile(typeof Buffer !== 'undefined' ? Buffer.from(str, 'base64') : Uint8Array.from(atob(str), b => b.charCodeAt(0))); | ||
|
@@ -285,6 +401,8 @@ pub fn render_intrinsics( | |
const i64ToF64 = i => (i64ToF64I[0] = i, i64ToF64F[0]); | ||
"), | ||
|
||
Intrinsic::Imports => {}, | ||
|
||
Intrinsic::InstantiateCore => if !instantiation { | ||
output.push_str(" | ||
const instantiateCore = WebAssembly.instantiate; | ||
|
@@ -654,6 +772,14 @@ impl Intrinsic { | |
pub fn get_global_names() -> &'static [&'static str] { | ||
&[ | ||
// Intrinsic list exactly as below | ||
"asyncifyAssertNoneState", | ||
"asyncifyInstantiate", | ||
"asyncifyModules", | ||
"asyncifyPromise", | ||
"asyncifyResolved", | ||
"asyncifyState", | ||
"asyncifyWrapExport", | ||
"asyncifyWrapImport", | ||
"base64Compile", | ||
"clampGuest", | ||
"ComponentError", | ||
|
@@ -671,6 +797,7 @@ impl Intrinsic { | |
"hasOwnProperty", | ||
"i32ToF32", | ||
"i64ToF64", | ||
"imports", | ||
"instantiateCore", | ||
"isLE", | ||
"resourceCallBorrows", | ||
|
@@ -733,6 +860,10 @@ impl Intrinsic { | |
|
||
pub fn name(&self) -> &'static str { | ||
match self { | ||
Intrinsic::AsyncifyAsyncInstantiate => "asyncifyInstantiate", | ||
Intrinsic::AsyncifySyncInstantiate => "asyncifyInstantiate", | ||
Intrinsic::AsyncifyWrapExport => "asyncifyWrapExport", | ||
Intrinsic::AsyncifyWrapImport => "asyncifyWrapImport", | ||
Intrinsic::Base64Compile => "base64Compile", | ||
Intrinsic::ClampGuest => "clampGuest", | ||
Intrinsic::ComponentError => "ComponentError", | ||
|
@@ -751,6 +882,7 @@ impl Intrinsic { | |
Intrinsic::HasOwnProperty => "hasOwnProperty", | ||
Intrinsic::I32ToF32 => "i32ToF32", | ||
Intrinsic::I64ToF64 => "i64ToF64", | ||
Intrinsic::Imports => "imports", | ||
Intrinsic::InstantiateCore => "instantiateCore", | ||
Intrinsic::IsLE => "isLE", | ||
Intrinsic::ResourceCallBorrows => "resourceCallBorrows", | ||
|
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.
I think we should be a bit smarter about this memory detection, and instead make the
memoryExportName
an argument into theasyncifyInstantiate
function - we should be able to have the context at the Rust side to know this information?Also flipping between the imports or the exports for the memory seems a bit too loose - can you explain the cases further? Could we make it a boolean argument from the Rust side whether the import or export memory should be used?