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 for release builds #1337

Merged
merged 1 commit into from
Dec 1, 2023
Merged

Enable LTO for release builds #1337

merged 1 commit into from
Dec 1, 2023

Conversation

Swatinem
Copy link
Member

@Swatinem Swatinem commented Nov 30, 2023

Related to #1334 CC @zamazan4ik

This is so far just doing the first step, using 1 CGU and LTO to optimize the compiler output, without relying on instrumentation and gathering profiling.

Using cargo --timings, I get local bound times of ~260s of Rust build + ~640s of linking time (LLVM does code generation at link time with LTO if I understand things correctly) for a total of ~15 minutes.

The first part we can mostly solve with dependency caching, but not the last linking step. So our docker builds will most likely take in the order of 10 minutes.

I would actually like to land this and maybe let it run over the weekend in a canary, so we can compare that to the baseline.

#skip-changelog

@Swatinem Swatinem self-assigned this Nov 30, 2023
Copy link

codecov bot commented Nov 30, 2023

Codecov Report

Merging #1337 (7d6b2ae) into master (8f3a697) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1337      +/-   ##
==========================================
- Coverage   75.67%   75.67%   -0.01%     
==========================================
  Files          99       99              
  Lines       14765    14767       +2     
==========================================
+ Hits        11174    11175       +1     
- Misses       3591     3592       +1     

@zamazan4ik
Copy link

zamazan4ik commented Nov 30, 2023

If building time is important, I can suggest a trick with creating an additional Cargo profile (let's call it profile.release-lto) and build it not per every commit but maybe per release or something like that.

Here are some examples:

Another positive outcome from enabling LTO is reduced binary size (however for Symbolicator it shouldn't be a critical advantage).

@zamazan4ik
Copy link

~640s of linking time

Regarding linking time. I can suggest you switch to LLD or even the Mold linker. Here are some benchmarks between Gold vs LLD vs Mold.

If you decide to use LLD and want to optimize the CI pipeline as much as possible, I can suggest recompiling LLD with PGO - the results can be found here.

@zamazan4ik
Copy link

@Swatinem also I can suggest enabling LTO for other Sentry projects like https://github.com/getsentry/relay

@Swatinem Swatinem merged commit 43c66a9 into master Dec 1, 2023
14 of 15 checks passed
@Swatinem Swatinem deleted the swatinem/lto branch December 1, 2023 09:58
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