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

cargo: Enable LTO on release build #2141

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jpds
Copy link

@jpds jpds commented Feb 9, 2024

No description provided.

@bluejekyll
Copy link
Member

I think I fixed the cleanliness issues. In terms of LTO, I used to see some issues with it functioning properly. Can you do us a favor, and list out the reasons for wanting it enabled? It would also be great to show before and after binary sizes to show the value here.

@jpds
Copy link
Author

jpds commented Mar 11, 2024

In terms of LTO, I used to see some issues with it functioning properly.

Do you remember any details on this? I've had LTO enabled on a number of Rust-based projects on my systems for some time now and haven't noticed any issues with them. Including the VMM I use for my VM infra.

Can you do us a favor, and list out the reasons for wanting it enabled?

Basically just to have the linker produce optimized binaries.

It would also be great to show before and after binary sizes to show the value here.

Before:

.rwxr-xr-x@ 6.3M user 11 Mar 22:04 dns
.rwxr-xr-x@  10M user 11 Mar 22:04 hickory-dns
.rwxr-xr-x@ 7.2M user 11 Mar 22:04 recurse
.rwxr-xr-x@ 6.9M user 11 Mar 22:04 resolve

After:

.rwxr-xr-x@ 2.8M user 11 Mar 22:09 dns
.rwxr-xr-x@ 4.6M user 11 Mar 22:09 hickory-dns
.rwxr-xr-x@ 2.9M user 11 Mar 22:09 recurse
.rwxr-xr-x@ 3.0M user 11 Mar 22:09 resolve

@bluejekyll
Copy link
Member

Thank you, I appreciate the follow up. Will merge, and thanks for the PR!

[profile.release]
lto = true
codegen-units = 1
opt-level = "s"
Copy link
Member

Choose a reason for hiding this comment

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

Do we want s or 3 here? I'm a little uncomfortable with optimizing for size, when I think we in most cases want to optimize for speed, i.e. 3. It seems like if people want to override the project default they can pretty easily?

lto = true
codegen-units = 1
opt-level = "s"
strip = true
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer this to be symbols to be clearer which strip mode is being enabled.

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.

None yet

3 participants