Use WebAssembly.Function constructor if available #9908
Conversation
|
Thank you for submitting a pull request! If this is your first PR, make sure to add yourself to AUTHORS. |
With the type reflection proposal, we will get a way more efficient way to convert a JS function into a wasm exported function. Use that if available.
|
Awesome! Thanks. We should probably try to test this. Is there a nodejs flag that we can set in a unit test? I wonder if we use a recent enough node during testing? We currently use 12.9.1. Perhaps we could have new flag such as ASSUME_LATEST_FEATURES.. that would allows us to compile out the legacy support and enable the testing of this codepath. |
| function convertJsFunctionToWasm(func, sig) { | ||
| #if WASM2JS | ||
| return func; | ||
| #else // WASM2JS | ||
|
|
||
| // If the type reflection proposal is available, use the new | ||
| // "WebAssembly.Function" constructor. | ||
| // Otherwise, construct a minimal wasm module importing the JS function and |
There was a problem hiding this comment.
Can this be made into a general polyfill?
There was a problem hiding this comment.
At which level are you imagining this polyfill? Do you suggest implementing a 'WebAssembly.Function' function if it's not defined? That would mostly work, but the returned object would still be of different type I think.
There was a problem hiding this comment.
Yeah, that's what I thought. I'm not sure specific returned type matters too much (especially in this codebase), but if it does, we can always do setPrototypeOf on the returned function.
There was a problem hiding this comment.
What would be the benefit of doing this? Less code in the convertJsFunctionToWasm function?
There was a problem hiding this comment.
Yeah, but also I thought it might be useful as a generic polyfill for other users that Emscripten just happens to use.
|
V8 has the --wasm-staging flag, which enables type reflection (including WebAssembly.Function). Node would inherit that flag. If possible, I think it would generally be good to also test with --wasm-staging enabled, since these are the features that we plan to enable by default soon. |
|
Thanks @backes ! Code lgtm. The CI test failure looks unrelated, but it's blocking the relevant tests from running, so we'll need to figure that out. Do you know what the standardization stage of |
|
Tests look good, thanks again @backes! |
With the type reflection proposal, we will get a way more efficient way to convert a JS function into a wasm exported function. Use that if available.
With the type reflection proposal, we will get a way more efficient way
to convert a JS function into a wasm exported function. Use that if
available.