-
Notifications
You must be signed in to change notification settings - Fork 70
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
Add optimize sub-command using wasm-opt #236
Conversation
I have a concern about merging this:
I am not attached to merging / adding this if folks think it isn't a good idea. |
|
You already depend on |
I have published version 0.110.2 of this crate that contains a single Binaryen backport that slightly reduces output size when the input module contains no "memories" section. |
Ah nice. @brson Thank you! |
Initially, I'm a bit on the fence about this. Argument for including the
Argument for excluding the
But, overall, from writing that down, I think this SGTM. |
For npm projects this makes sense. For Rust projects, there's no way to hook post-build steps into a project. The reason to include But maybe it is being too opinionated.
Good point. Your comment also got me thinking that automatically modifying contracts during deploy could be surprising, and if there was ever a bug there's no opportunity for a developer to test out the artifact before it deploys. |
I did some tests with this and it appears no. I managed to get a .wasm file even smaller by running it multiple times. |
@leighmcculloch there is a |
Thanks @willemneal. I'm experimenting with that option in #238. |
What
Add optimize sub-command using wasm-opt.
Why
I saw @brson and @Aimeedeer's blog post about the
wasm-opt
crate today and after a quick test it appears to work very smoothly. Including optimization into the soroban-cli rather than instructing people to install other tools seems like it fits our philosophy of making writing effecient contracts the default.This PR introduces an ultra simple sub-command (
soroban optimize
) that runswasm-opt
across the given .wasm file with the-Oz
mode, which is the same mode we currently recommend.I imagine we might also want to automatically run this across .wasm files as part of the deploy subcommand, although I thought doing that at this time, given that we are considering redesigning the CLI (#197), might be a bit premature.