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

Reduce size of release binary #592

Merged
merged 4 commits into from
Feb 20, 2024

Conversation

andrews05
Copy link
Collaborator

@andrews05 andrews05 commented Feb 18, 2024

This PR makes 3 changes that together reduce binary size by around 25%:

  • Sets lto="fat" in cargo.toml
  • Sets panic="abort" in cargo.toml
  • Sets location-detail=none in RUSTFLAGS

Closes #571

An unrelated change: I've replaced the zopfli test file with a smaller one that runs much faster, as well as removing the slow test for issue-133 which was related to an older alpha optimisation that is no longer relevant.

@andrews05 andrews05 marked this pull request as ready for review February 18, 2024 06:43
@ace-dent
Copy link

@andrews05 - thanks for implementing this! Is the new image for testing permissively licensed?
Comments based on my recollection of #571 ...

  1. You decided to avoid opt-level=s, which reduced execution speed ~10%. Now release will use the default 3 (it hasn't been explicitly stated). Seems reasonable.
  2. In addition to lto=fat, I think it is useful to include codegen-units=1. This is missing?
  3. I believe build-std with panic_immediate_abort were also found to be useful ?

Many thanks :-)

@andrews05
Copy link
Collaborator Author

The image is just a copy of another one in the test suite (see #129 - it comes from a corpus of png test images).

  1. codegen-units=1 drastically slows down compile time for negligible benefit to file size, so it really doesn't seem worth it.
  2. build-std is a much more advanced technique that I would definitely not recommend for a typical build process, unless there was a very specific need to reduce file size as much as possible. Since we don't have such a need, I'd much rather leave it up to end users to perform this if it's important to them.

@ace-dent
Copy link

Totally understand your points but it's a shame to not adopt (3), which seemed to save ~30% binary size.

@andrews05
Copy link
Collaborator Author

Yeah, I agree. It's a shame that rust can't avoid the bloat more easily.

Copy link
Collaborator

@AlexTMjugador AlexTMjugador left a comment

Choose a reason for hiding this comment

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

I think these changes look good, thanks!

Given that we're already using a nightly toolchain for the release builds, I don't think going with build-std is that difficult, but it surely comes with the tradeoff of slower CI times. We can explore that option in more depth in the future; the changes in this PR are not mutually exclusive with that.

@AlexTMjugador AlexTMjugador merged commit 98fd4de into shssoichiro:master Feb 20, 2024
10 checks passed
@andrews05 andrews05 deleted the feature/bin-size branch February 20, 2024 19:13
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.

Optimize file size of release binaries
3 participants