Skip to content
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

Support polyfilling multi-memory #125

Merged

Conversation

alexcrichton
Copy link
Member

Currently JS does not generally support the WebAssembly multi-memory
proposal. Wasmtime will, however, generate adapter modules which use
multi-memory to communicate between components. This means that composed
components are typically not compatible with the transpile process as
they produce a core module that doesn't actually run in any JS runtime.

This commit fixes this issue by adding polyfill support for
multi-memory. Whenever an adapter is produced that uses multi-memory jco
will now rewrite the module such that any references to memory that
isn't at index 0 to be indirected through functions. These imported
functions then operate on the specified memory on behalf of the wasm
itself. This is a horribly slow process because all memory reads/writes
become function calls, but given that the baseline is otherwise "does
not work" it's hopefully a bit better than before.

The end goal here is to work up towards strings between components, but
for now this just gets everything else working with multi-memory such as
transferring lists.

This commit updates the Wasmtime dependency to the, currently
unreleased, Wasmtime 12.0.0 branch. A number of changes happened to
component translation in Wasmtime 12 to account for resources which
required changes here as well. Additionally Wasmtime 12 disallows empty
types in components which WASI was previously using, so this
additionally updates the preview1 shims and test components.
Currently JS does not generally support the WebAssembly multi-memory
proposal. Wasmtime will, however, generate adapter modules which use
multi-memory to communicate between components. This means that composed
components are typically not compatible with the transpile process as
they produce a core module that doesn't actually run in any JS runtime.

This commit fixes this issue by adding polyfill support for
multi-memory. Whenever an adapter is produced that uses multi-memory jco
will now rewrite the module such that any references to memory that
isn't at index 0 to be indirected through functions. These imported
functions then operate on the specified memory on behalf of the wasm
itself. This is a horribly slow process because all memory reads/writes
become function calls, but given that the baseline is otherwise "does
not work" it's hopefully a bit better than before.

The end goal here is to work up towards strings between components, but
for now this just gets everything else working with multi-memory such as
transferring lists.
@alexcrichton
Copy link
Member Author

I'll note that this is based on #124 because Wasmtime 12 has a minor fix required to get this working.

Copy link
Collaborator

@guybedford guybedford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic, I've been worried about this. Will try it out first thing tomorrow.

crates/js-component-bindgen/src/core.rs Outdated Show resolved Hide resolved
crates/js-component-bindgen/src/transpile_bindgen.rs Outdated Show resolved Hide resolved
//! support for more instructions but such support isn't required at this time.
//! Examples of unsupported instructions are `i64.load8_u` and `memory.copy`.
//! Additionally core wasm sections such as data sections and tables are not
//! supported because, again, Wasmtime doesn't use it at this time.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering if it might make sense to use a full wasm2js solution here (eg something like https://github.com/evanw/polywasm), but this certainly seems adequate as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think any of those would work myself, this was just the first thing I thought of so I figured I'd take a stab implementing it while stuck delayed in an airport :)

If you'd prefer to go with those though I think it's ok to hold this open or close it, either way's fine by me

@guybedford guybedford mentioned this pull request Aug 7, 2023
@guybedford guybedford merged commit 31d3eab into bytecodealliance:main Aug 8, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants