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

Enable LTO when profiling #1469

Merged
merged 3 commits into from
Feb 13, 2024
Merged

Enable LTO when profiling #1469

merged 3 commits into from
Feb 13, 2024

Conversation

rukai
Copy link
Member

@rukai rukai commented Feb 11, 2024

The bench profile is a default profile that already exists that gets used when running benches.
It inherits from release and all the config that we set for it is already set by release.
So I just deleted it.

The profiling profile is a profile derived from release that windsock uses to recompile shotover to ensure that it has debuginfo enabled so that profiling it will give useful results.
The magic line that enables this is debug = true but previously I also thought that we needed to disable LTO.
However it turns out that it works just fine with LTO enabled.
So this PR enables lto by removing the lto = false line and letting it fall back to the release profiles lto = fat
This is very useful as it ensures that we are profiling a binary that is a lot closer to what we release.

I also removed the codegen-units = 1 line since that is already set in the release profile that we derive from.

I did attempt to combine the profiling and release profiles, so that we are profiling the same binary we release.
However with debug = true the binary increases to 186MB from 20MB, this is way too big.
If I then enable split-debuginfo it reduces to 40MB but that is still too large.
So with the current state of things I dont we think we should combine them.

@rukai rukai requested a review from conorbros February 11, 2024 22:59
@rukai rukai enabled auto-merge (squash) February 13, 2024 20:03
@rukai rukai merged commit 4a81275 into shotover:main Feb 13, 2024
40 checks passed
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.

3 participants