Cargo: hints.min-opt-level

[joshtriplett]

This RFC adds a mechanism for crates to hint that builds should use
optimization by default, while still allowing the top-level crate to
easily override this.

Important

When responding to RFCs, try to use inline review comments (it is possible to leave an inline review comment for the entire file at the top) instead of direct comments for normal comments and keep normal comments for procedural matters like starting FCPs.

This keeps the discussion more organized.

Rendered

[joshtriplett]

---

I've incorporated all feedback as of this comment.

[epage]

Can we call out that we should document overriding dependency opt-level when debugging?

[joshtriplett]

Would love to get feedback from crates that are likely to want this.

cc @alice-i-cecile for Bevy.

cc @abonander for sqlx.

[AMDmi3]

Maybe also @LaurentMazare for candle (the perf difference is so large it's likely pointless to use unoptimized candle as a dependency), and @mitsuhiko for insta (insta suggests to set opt-level, though I've never in fact witnessed even a slight difference).

[epage]

An interesting case is rsa which has

Note: If you encounter unusually slow key generation time while using RsaPrivateKey::new you can try to compile in release mode or add the following to your Cargo.toml. Key generation is much faster when building with higher optimization levels, but this will increase the compile time a bit.

[profile.debug]
opt-level = 3

If you don't want to turn on optimizations for all dependencies,
you can only optimize the num-bigint-dig dependency. This should
give most of the speedups.

[profile.dev.package.num-bigint-dig]
opt-level = 3

Though that is removed for the next version (RustCrypto/RSA#523) because they switched dependencies.

CC @tarcieri

[abonander]

I was unable to un-resolve the previous thread myself so I'm just starting a new one instead:

I would definitely be hesitant to set this by default in SQLx because I can easily see people coming to us complaining that it's suddenly building a lot slower. Clean build performance doesn't normally affect my workflow, at least for local development builds, but one place it does come up a lot is in CI.

Caching is finicky to set up and configure, can be really flaky and is nearly impossible to debug when it is, and when using the actions/cache action or any actions built on it like swatinem/rust-cache, it can be a lot slower on self-hosted runners because of throttling. I've actually seen build times go down after deleting the caching step simply because it was bottlenecking on uploading the archive (sometimes even to the point of timing out).

It would obviously be a major behavior change, but I've been thinking for a while that users who care about performance in dev builds should have the option to just build all dependencies with optimizations+debuginfo by default. I started opining on this until I realized that this already existed, which I didn't know before because it's only mentioned in passing in one spot in the Cargo book:

The precedence for which value is used is done in the following order (first match wins):

1. [profile.dev.package.name] — A named package.
2. [profile.dev.package."*"] — For any non-workspace member.
3. ...

[epage]

I've been thinking for a while that users who care about performance in dev builds should have the option to just build all dependencies with optimizations+debuginfo by default.

We're tracking this at rust-lang/cargo#15931

I started opining on this until I realized that this already existed, which I didn't know before because it's only mentioned in passing in one spot in the Cargo book:

We also talk about that config at https://doc.rust-lang.org/nightly/cargo/guide/build-performance.html#reduce-amount-of-generated-debug-information but only for debug info and not optimizations

[epage]

I would definitely be hesitant to set this by default in SQLx because I can easily see people coming to us complaining that it's suddenly building a lot slower. Clean build performance doesn't normally affect my workflow, at least for local development builds, but one place it does come up a lot is in CI.

Your docs mention opt-level 3. What is the incremental improvement between opt-level 0, 1, and 3? If 3 is noticeable enough, I wonder if a min-opt-level of 1 would strike a good balance.

[abonander]

I think it'd be all right if Cargo emitted a message informing the user that an override was being applied automatically here, but then again if it did that for every crate in the graph that specified an override then it'd just get lost in the noise.

However, I definitely feel that "somehow educate the user and let them make the decision" would be the most Rust-y way to do this.

[abonander]

Your docs mention opt-level 3. What is the incremental improvement between opt-level 0, 1, and 3? If 3 is noticeable enough, I wonder if a min-opt-level of 1 would strike a good balance.

That advice is specifically for speeding up the proc macros, since they're so much more heavyweight than pretty much anything else out there. It can really have an outsized effect on compile times.

It's definitely dependent on the project, but for this example project which uses a number of query macros as well as #[sqlx::test] invocations, cargo build with a fully cached dep tree goes from 2.7 seconds to 2.6 to 2.4 for profile.dev.package."*".opt-level = 0, 1 and 3 respectively.

Just setting profile.dev.build-override.opt-level seems to be more of a wash, though. When I tried 0, 1 and 3 with the same procedure (cargo clean && cargo build -q && cargo clean -p sqlx-example-postgres-axum-social && cargo build), I got 2.55s, 2.73s and 2.74s, respectively.

The differences here are not exactly something a human would notice anyway, but I just don't have a huge project using SQLx close to hand anymore since I started my new job.

[obi1kenobi]

Would love to get feedback from crates that are likely to want this.

cargo-semver-checks and its Trustfall dependency both would benefit from a hint that recommends enabling optimizations even in debug profiles.

While I can't seem to find the concrete issue, I recall at one point @epage having a performance concern about cargo-semver-checks being very slow that turned out to be due to unoptimized builds causing poor Iterator performance when Trustfall and cargo-semver-checks use iterators very heavily.

A hint to set a minimum optimization level would have prevented that problem entirely.

Ideally, we'd be able to neatly specify "trustfall + selected dependencies" should be optimized, since there are often multiple crates in play: trustfall, trustfall_core, the adapter which is usually a separate crate (e.g. trustfall-rustdoc-adapter), etc. But I wouldn't let perfect be the enemy of "good enough and shipped" here :)

I have not benchmarked the differences between opt-level being set to 1 / 2 / 3. From a downstream user perspective, trustfall and related dependencies only get compiled ~once, so set-and-forget to 3 has been fine so far IMHO. We have not needed s / z optimization modes thus far.

[tarcieri]

Something like this would definitely be useful, as we inevitably get asked about why things are slow in debug builds, e.g. RustCrypto/RSA#144