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

LTO and CGU in CI dist build for cargo itself #14719

Open
weihanglo opened this issue Oct 23, 2024 · 4 comments
Open

LTO and CGU in CI dist build for cargo itself #14719

weihanglo opened this issue Oct 23, 2024 · 4 comments
Labels
A-building-cargo-itself Area: issues with building cargo Performance Gotta go fast! S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@weihanglo
Copy link
Member

weihanglo commented Oct 23, 2024

Inspired by @x-hgg-x 's recent works, I just did some benchmarks setting [profile.release] for Cargo. While with LTO on Cargo is 1.05x faster, the benchmark result doesn't seem good enough to convince myself to add an extra 3 minutes build time.

Opened this issue just for a record.


This benchmark is based on 42f4143

  • master — no change in [profile.release]
  • fat-1 — lto=fat + codegen-units=1
  • fat-16 — lto=fat + codegen-units=16 (default of release profile)
  • thin-1 — lto=thin + codegen-units=1
  • thin-16 — lto=thin + codegen-units=16 (default of release profile)
Command Build time Binary size
master 1m 04s 36M
fat-1 4m 16s 28M
fat-16 3m 27s 29M
thin-1 3m 36s 31M
thin-16 1m 08s 37M

With cargo dependencies + aws-sdk-ec2

This benchmarks dependency resolution.

cargo generate-lockfile -Zno-index-update

Command Mean [ms] Min [ms] Max [ms] Relative
master 482.9 ± 2.4 477.1 486.5 1.06 ± 0.01
fat-1 456.8 ± 1.8 452.7 459.9 1.00 ± 0.01
fat-16 461.7 ± 1.4 459.4 464.3 1.01 ± 0.00
thin-1 456.4 ± 1.7 453.1 460.0 1.00
thin-16 474.9 ± 1.9 470.9 479.4 1.04 ± 0.01

With cargo dependencies

This benchmarks no-op build (Cargo overhead before rustc compilation).

cargo check (prefilled cached with cargo check)

Command Mean [ms] Min [ms] Max [ms] Relative
master 226.5 ± 0.6 225.4 227.5 1.05 ± 0.00
fat-1 215.6 ± 0.8 214.0 216.7 1.00
fat-16 216.7 ± 1.2 213.9 218.6 1.01 ± 0.01
thin-1 217.5 ± 0.7 216.1 218.6 1.01 ± 0.00
thin-16 222.6 ± 0.5 221.6 223.6 1.03 ± 0.00

With cargo dependencies

This benchmarks unpacking .crates files and no-op build.

rm -rf ~/.cargo/registry/src && cargo check

Command Mean [s] Min [s] Max [s] Relative
aster 2.270 ± 0.024 2.240 2.326 1.01 ± 0.02
fat-1 2.251 ± 0.017 2.219 2.286 1.00 ± 0.01
fat-16 2.248 ± 0.025 2.218 2.309 1.00
thin-1 2.264 ± 0.036 2.230 2.360 1.01 ± 0.02
thin-16 2.262 ± 0.025 2.238 2.314 1.01 ± 0.02
@weihanglo weihanglo added A-building-cargo-itself Area: issues with building cargo Performance Gotta go fast! labels Oct 23, 2024
@weihanglo
Copy link
Member Author

This benchmark doesn't cover Cargo target discovery and other parts, though.

@KGrewal1
Copy link

KGrewal1 commented Oct 23, 2024

Would a separate dist and release profile make sense (so you pay the cost of the added time less frequently)? Think this pattern is somewhat common in other repos using cargo publish? (Also feels worth considering that this is acc saving 26s on the benchmarks, so if it ran 10 times, that's more than made up for the extra time compiling cargo)

@weihanglo
Copy link
Member Author

weihanglo commented Oct 23, 2024

I am not sure from which angle you're looking at, but yeah a separate profile may make sense.

The distribution of Cargo is done along with the Rust toolchain in rust-lang/rust. There are some CI jobs in rust-lang/rust build release mode Cargo and verify its behavior.
Not sure if they reuse those artifacts to make the distribution tarball. I doubt it but if yes that means we may build Cargo twice for the distribution process.

@weihanglo
Copy link
Member Author

To make it clear in rust-lang/cargo we don't publish any crate to crates.io. It is the rust-lang release team doing it separately.

See https://doc.crates.io/contrib/process/release.html

@weihanglo weihanglo added the S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. label Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-building-cargo-itself Area: issues with building cargo Performance Gotta go fast! S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

No branches or pull requests

2 participants