-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
cargo bench
builds build dependencies in release mode
#2234
Comments
Example: crate depends on rustc_version which in turn depends on semver. Semver is built in release mode for benchmarks. |
I believe this is a more general issue of the release profile building build-dependencies in release mode, of which bench is one case. It's particularly annoying with things like syntex_syntax that already take a long time. |
I don't think there's a clear answer here of what Cargo should be doing. Most build dependencies probably don't need to be optimized, but that doesn't mean that none need to be optimized. It's unlikely that the optimization of the final artifacts should be related to the optimization of the build scripts |
In fact I would consider it a bug if the optimization of the final artifacts were related to the optimization of the build scripts. I agree with @bluss and @sfackler that build-dependencies never need to be optimized. |
Unfortunately others actually feel the opposite, where build dependencies like lalrpop should always be optimized as otherwise their execution is super slow. This is basically what I was just thinking before, however, where I don't think there's a clear answer for what Cargo should do. There should likely be some form of configuration option at least for this, however (in one form or another) |
I would love to drop 160 seconds from my release build time by letting Clippy build without optimizations. Clippy is fast enough, it's just slow to compile. And build caching is a bit flaky for me. |
I've ran into this too. I use bindgen at build time, and it takes ages to compile it release mode. An option to have separate profiles for build dependencies (similar to #1359) would be fantastic. |
Profile Overrides (RFC #2282 Part 1) Profile Overrides (RFC #2282 Part 1) WIP: Putting this up before I dig into writing tests, but should be mostly complete. I also have a variety of questions below. This implements the ability to override profiles for dependencies and build scripts. This includes a general rework of how profiles work internally. Closes #5298. Profile overrides are available with `profile-overrides` set in `cargo-features` in the manifest. Part 2 is to implement profiles in config files (to be in a separate PR). General overview of changes: - `Profiles` moved to `core/profiles.rs`. All profile selection is centralized there. - Removed Profile flags `test`, `doc`, `run_custom_build`, and `check`. - Removed `Profile` from `Unit` and replaced it with two enums: `CompileMode` and `ProfileFor`. This is the minimum information needed to compute profiles at a later stage. - Also removed `rustc_args`/`rustdoc_args` from `Profile` and place them in `Context`. This is currently not very elegant because it is a special case, but it works. An alternate solution I considered was to leave them in the `Profile` and add a special uber-override layer. Let me know if you think it should change. - Did some general cleanup in `generate_targets`. ## Misc Fixes - `cargo check` now honors the `--release` flag. Fixes #5218. - `cargo build --test` will set `panic` correctly for dependences. Fixes #5369. - `cargo check --tests` will no longer include bins twice (once as a normal check, once as a `--test` check). It only does `--test` check now. - Similarly, `cargo check --test name` no longer implicitly checks bins. - Examples are no longer considered a "test". (See #5397). Consequences: - `cargo test` will continue to build examples as a regular build (no change). - `cargo test --tests` will no longer build examples at all. - `cargo test --all-targets` will no longer build examples as tests, but instead build them as a regular build (now matches `cargo test` behavior). - `cargo check --all-targets` will no longer check examples twice (once as normal, once as `--test`). It now only checks it once as a normal target. ## Questions - Thumbs up/down on the general approach? - The method to detect if a package is a member of a workspace should probably be redone. I'm uncertain of the best approach. Maybe `Workspace.members` could be a set? - `Hash` and `PartialEq` are implemented manually for `Profile` only to avoid matching on the `name` field. The `name` field is only there for debug purposes. Is it worth it to keep `name`? Maybe useful for future use (like #4140)? - I'm unhappy with the `Finished` line summary that displays `[unoptimized + debuginfo]`. It doesn't actually show what was compiled. Currently it just picks the base "dev" or "release" profile. I'm not sure what a good solution is (to be accurate it would need to potentially display a list of different options). Is it ok? (See also #4140 for the wrong profile name being printed.) - Build-dependencies use different profiles based on whether or not `--release` flag is given. This means that if you want build-dependencies to always use a specific set of settings, you have to specify both `[profile.dev.build_override]` and `[profile.release.build_override]`. Is that reasonable (for now)? I've noticed some issues (like #1774, #2234, #2424) discussing having more control over how build-dependencies are handled. - `build --bench xxx` or `--benches` builds dependencies with dev profile, which may be surprising. `--release` does the correct thing. Perhaps print a warning when using `cargo build` that builds benchmark deps in dev mode? - Should it warn/error if you have an override for a package that does not exist? - Should it warn/error if you attempt to set `panic` on the `test` or `bench` profile? ## TODO - I have a long list of tests to add. - Address a few "TODO" comments left behind.
With profile overrides feature, which is stabilized in 1.41, build dependencies shouldn't be optimized by default. However, it wasn't true until a bug got fixed in 1.47 with #8560. If you want to optimize build dependencies for [profile.release.build-override]
opt-level = 3 Close as resolved in 1.47. |
Build deps are unrelated to the performance or benchmarking, so they don't need to be rebuilt, and don't need to be in release mode.
The text was updated successfully, but these errors were encountered: