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

dep: Optionalize wasm-opt crate dependency #63

Merged

Conversation

kateinoigakukun
Copy link
Contributor

@kateinoigakukun kateinoigakukun commented Jun 19, 2024

The wasm-opt crate brings in C++ dependencies (binaryen) and it makes the build process more complex and slower (also setting up cross-compilation for C++ is not trivial unlike Rust). Also the DCE process doesn't make much difference if we allow all WASI subsystems.
This commit makes the wasm-opt crate dependency optional so that crate users can choose whether to use it or not.

Also strip the "name" section at walrus module generation time if debug=false because it was stripped by wasm-opt before.

Data point

We use wasi-virt to embed Ruby installation package with --allow-all:

wasm-opt=true wasm-opt=false
28101333 28101048 (+285)

At least for us, more 300 bytes to simplify build is fine.

The `wasm-opt` crate brings in C++ dependencies (binaryen) and it makes
the build process more complex and slower (also setting up
cross-compilation for C++ is not trivial unlike Rust).
Also the DCE process doesn't make much difference if we allow all WASI
subsystems.
This commit makes the `wasm-opt` crate dependency optional so that crate
users can choose whether to use it or not.

Also strip the "name" section at walrus module generation time if
`debug=false` because it was stripped by `wasm-opt` before.
@guybedford guybedford merged commit 02de7b4 into bytecodealliance:main Jun 19, 2024
2 checks passed
@kateinoigakukun kateinoigakukun deleted the katei/feature-wasm-opt branch July 3, 2024 05:36
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