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

Migrate wasm-bindgen to using walrus #1237

Merged
merged 1 commit into from
Feb 12, 2019

Conversation

alexcrichton
Copy link
Contributor

This commit moves wasm-bindgen the CLI tool from internally using
parity-wasm for wasm parsing/serialization to instead use walrus.
The walrus crate is something we've been working on recently with an
aim to replace the usage of parity-wasm in wasm-bindgen to make the
current CLI tool more maintainable as well as more future-proof.

The walrus crate provides a much nicer AST to work with as well as a
structured Module, whereas parity-wasm provides a very raw interface
to the wasm module which isn't really appropriate for our use case. The
many transformations and tweaks that wasm-bindgen does have a huge
amount of ad-hoc index management to carefully craft a final wasm
binary, but this is all entirely taken care for us with the walrus
crate.

Additionally, wasm-bindgen will ingest and rewrite the wasm file,
often changing the binary offsets of functions. Eventually with DWARF
debug information we'll need to be sure to preserve the debug
information throughout the transformations that wasm-bindgen does
today. This is practically impossible to do with the parity-wasm
architecture, but walrus was designed from the get-go to solve this
problem transparently in the walrus crate itself. (it doesn't today,
but this is planned work)

It is the intention that this does not end up regressing any
wasm-bindgen use cases, neither in functionality or in speed. As a
large change and refactoring, however, it's likely that at least
something will arise! We'll want to continue to remain vigilant to any
issues that come up with this commit.

Note that the gc crate has been deleted as part of this change, as the
gc crate is no longer necessary since walrus does it automatically.
Additionally the gc crate was one of the main problems with preserving
debug information as it often deletes wasm items!

Finally, this also starts moving crates to the 2018 edition where
necessary since walrus requires the 2018 edition, and in general it's
more pleasant to work within the 2018 edition!

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Not too shabby!

crates/wasm-interpreter/src/lib.rs Show resolved Hide resolved
@fitzgen
Copy link
Member

fitzgen commented Feb 12, 2019

+990 −3,531

Nice, looks like this is actually paying off :)

This commit moves `wasm-bindgen` the CLI tool from internally using
`parity-wasm` for wasm parsing/serialization to instead use `walrus`.
The `walrus` crate is something we've been working on recently with an
aim to replace the usage of `parity-wasm` in `wasm-bindgen` to make the
current CLI tool more maintainable as well as more future-proof.

The `walrus` crate provides a much nicer AST to work with as well as a
structured `Module`, whereas `parity-wasm` provides a very raw interface
to the wasm module which isn't really appropriate for our use case. The
many transformations and tweaks that wasm-bindgen does have a huge
amount of ad-hoc index management to carefully craft a final wasm
binary, but this is all entirely taken care for us with the `walrus`
crate.

Additionally, `wasm-bindgen` will ingest and rewrite the wasm file,
often changing the binary offsets of functions. Eventually with DWARF
debug information we'll need to be sure to preserve the debug
information throughout the transformations that `wasm-bindgen` does
today. This is practically impossible to do with the `parity-wasm`
architecture, but `walrus` was designed from the get-go to solve this
problem transparently in the `walrus` crate itself. (it doesn't today,
but this is planned work)

It is the intention that this does not end up regressing any
`wasm-bindgen` use cases, neither in functionality or in speed. As a
large change and refactoring, however, it's likely that at least
something will arise! We'll want to continue to remain vigilant to any
issues that come up with this commit.

Note that the `gc` crate has been deleted as part of this change, as the
`gc` crate is no longer necessary since `walrus` does it automatically.
Additionally the `gc` crate was one of the main problems with preserving
debug information as it often deletes wasm items!

Finally, this also starts moving crates to the 2018 edition where
necessary since `walrus` requires the 2018 edition, and in general it's
more pleasant to work within the 2018 edition!
@alexcrichton alexcrichton merged commit 6004454 into rustwasm:master Feb 12, 2019
@alexcrichton alexcrichton deleted the walrus branch February 12, 2019 15:26
@fitzgen fitzgen added the TWiRaWA Nominate this PR for inclusion in the next issue of This Week in Rust and WebAssembly label Feb 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TWiRaWA Nominate this PR for inclusion in the next issue of This Week in Rust and WebAssembly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants