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

Disable debug info compilation in CI to boost performance. #779

Merged
merged 3 commits into from
Dec 9, 2024

Conversation

xStrom
Copy link
Member

@xStrom xStrom commented Dec 9, 2024

Turning off debug info in CI gives us tiny speedups, with potential for more in some scenarios. Most notably there is 30% space savings on artifact size in Linux, really helping with cache pressure and compression/decompression time.

We don't really need the benefits of debug info in the CI environment. We're not attaching debuggers there and in case of panics we can still get a backtrace. Thus it's just wasted effort to generate the debug info and then to link it in and cache it.

Benchmarks

I did some minor benchmarking on three different OS setups with Rust 1.83. Every step was repeated four times (1 warmup, then 3 for measuring), but stuff like CPU boost frequency etc was not controlled for. Even so, the numbers are useful for ballparking.

I set CARGO_INCREMENTAL to 0 to match the CI and measured cargo clippy -p xilem --locked --profile ci --all-features.

Windows 11 23H2 Old New Diff
Cold duration 17.40s 17.21s -1%
Warm duration 0.74s 0.73s -1%
Cache size 490 801 867 485 287 057 -1%
macOS 10.15.4 Old New Diff
Cold duration 30.58s 30.34s -1%
Warm duration 1.27s 1.21s -5%
Cache size 207 200 857 200 717 130 -3%
Ubuntu 23.10 Old New Diff
Cold duration 24.50s 23.50s -4%
Warm duration 0.84s 0.87s +4%
Cache size 597 456 507 411 918 698 -31% 🚀

Copy link
Member

@DJMcNab DJMcNab left a comment

Choose a reason for hiding this comment

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

Smaller cache sizes are great! It's especially important for the snapshots LFS workaround

Cargo.toml Outdated
@@ -128,5 +128,7 @@ time = "0.3.36"

[profile.ci]
inherits = "dev"
debug = 0 # Don't compile debug info to reduce compilation artifact size for cache benefits.
strip = "debuginfo" # Implied with debug = 0 since Rust 1.77, still useful for older.
Copy link
Member

Choose a reason for hiding this comment

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

This sentence doesn't scan to me - older doesn't really work as a subject. Maybe something like:

Suggested change
strip = "debuginfo" # Implied with debug = 0 since Rust 1.77, still useful for older.
strip = "debuginfo" # Implied by debug = 0 since Rust 1.77 - but our MSRV is older.

Copy link
Member

Choose a reason for hiding this comment

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

Nice!

Copy link
Member Author

Choose a reason for hiding this comment

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

It's meant to be copy-pastable and in Xilem's case the MSRV is actually newer than 1.77. I adjusted the comment but made the MSRV reference a bit more conditional.

Copy link
Contributor

Choose a reason for hiding this comment

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

MSRV will be 1.82 as soon as this gets updated Peniko or Vello.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should just not have the comment at all? The redundancy doesn't have any real cost

@xStrom xStrom added this pull request to the merge queue Dec 9, 2024
Merged via the queue into linebender:main with commit 0c2730d Dec 9, 2024
17 checks passed
@xStrom xStrom deleted the ciperf branch December 9, 2024 12:19
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