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

Change default optimizations pass to focus on code size #305

Merged
merged 3 commits into from
Jul 22, 2021

Conversation

HCastano
Copy link
Contributor

@HCastano HCastano commented Jul 19, 2021

This PR changes the default optimization option passed to wasm-opt from -O3 to -Os-Oz.
This saves us around 0.2-0.3K according to these rough benchmarks.

The default wasn't set to -Oz as I saw no noticeable size improvements, and it just
takes more time to run. However, you could also argue that since contracts are in general
quite small the extra time for -Oz is negligible so we should enable it anyways.

While I could be convinced of this, I'm not sure what hardware other people build on, and
the negative impact the change to -Oz could have for them.

@HCastano HCastano added the enhancement New feature or request label Jul 19, 2021
@athei
Copy link
Contributor

athei commented Jul 20, 2021

We should invest everything in making contracts as small as possible. Any contract above 50KB wouldn't be viable to run on a parachain anyways. So the optimization time should be negligible. In favor of setting it to -Oz.

@HCastano HCastano changed the title Change default opitmization pass to focus on code size Change default optimizations pass to focus on code size Jul 20, 2021
Copy link
Collaborator

@cmichi cmichi left a comment

Choose a reason for hiding this comment

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

Thank you!

Could you add a ### Changed subsection to the Unreleased section of our CHANGELOG.md: https://github.com/paritytech/cargo-contract/blob/master/CHANGELOG.md ?

@HCastano HCastano requested a review from cmichi July 21, 2021 19:03
@HCastano
Copy link
Contributor Author

Thank you!

Could you add a ### Changed subsection to the Unreleased section of our CHANGELOG.md: https://github.com/paritytech/cargo-contract/blob/master/CHANGELOG.md ?

I like that a CHANGELOG is kept. However, there's gotta be a way to automate this, no? Maybe @s3krit can share some of his automation secrets 👀

@cmichi cmichi merged commit f165b8c into master Jul 22, 2021
@cmichi cmichi deleted the hc-bump-size-opt branch July 22, 2021 03:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants