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

Don't spend build time optimizing build-time-only crates #8406

Conversation

joshtriplett
Copy link
Member

Crates used exclusively at build time, such as proc-macro crates and
their dependencies, don't benefit substantially from optimization; they
take far longer to optimize than time saved when running them during the
build. No machine code from these crates will appear in the final
compiled binary.

Use the new profile-overrides mechanism in Rust 1.41
(https://doc.rust-lang.org/cargo/reference/profiles.html#overrides) to
build such crates with opt-level 0.

Before:
Finished release [optimized] target(s) in 4m 20s
After:
Finished release [optimized] target(s) in 4m 07s

Crates used exclusively at build time, such as proc-macro crates and
their dependencies, don't benefit substantially from optimization; they
take far longer to optimize than time saved when running them during the
build. No machine code from these crates will appear in the final
compiled binary.

Use the new profile-overrides mechanism in Rust 1.41
(https://doc.rust-lang.org/cargo/reference/profiles.html#overrides) to
build such crates with opt-level 0.

Before:
    Finished release [optimized] target(s) in 4m 20s
After:
    Finished release [optimized] target(s) in 4m 07s
@rust-highfive
Copy link

r? @Eh2406

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 24, 2020
@Eh2406
Copy link
Contributor

Eh2406 commented Jun 25, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Jun 25, 2020

📌 Commit 78e7c01 has been approved by Eh2406

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 25, 2020
@bors
Copy link
Contributor

bors commented Jun 25, 2020

⌛ Testing commit 78e7c01 with merge a8723de370999bafcaaa8cca43ee0dd3e75dd793...

@ehuss
Copy link
Contributor

ehuss commented Jun 25, 2020

@bors r-
I would prefer not to do this. When embedded in the rust-lang/rust workspace, it would constantly emit warnings for everyone (#8264). It also would have no effect execpt for people locally build cargo in release mode, which I suspect is extremely rare.

@bors bors added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 25, 2020
@bors
Copy link
Contributor

bors commented Jun 25, 2020

☀️ Try build successful - checks-azure
Build commit: a8723de370999bafcaaa8cca43ee0dd3e75dd793 (a8723de370999bafcaaa8cca43ee0dd3e75dd793)

@joshtriplett
Copy link
Member Author

Ah, I see.

I do find this useful for local testing, but I wouldn't want to have rust-lang/rust warn about it. So until Cargo gets a mechanism for ignoring the profile warning (as suggested in the linked issue), feel free to ignore.

@Mark-Simulacrum
Copy link
Member

It may be worth exploring whether this makes sense as a heuristic for Cargo in general (rather than just applying it to this repository). I don't know how we'd evaluate such a change beyond checking a few well known projects, though.

I'm also not sure how much the 13s win here is meaningful - presumably incremental builds get slowed down by this change? Have you checked the effects there?

@ehuss
Copy link
Contributor

ehuss commented Jun 26, 2020

In #6577 I explored changing the defaults for build dependencies. That PR was a little more ambitious, in that it uses the same settings across dev and release. It was sometimes slower and sometimes faster. The big problem is that shared dependencies have to be built multiple times (which in some cases is a huge number of extra crates). Although if you use --target (like rust-lang/rust does), it is already building them twice. I don't recall doing incremental profiling (we talked about it though), I should do that when I resume that work.

@ehuss
Copy link
Contributor

ehuss commented Jul 1, 2020

I'm going to close this per the discussion above. Perhaps at some point in the future the warnings will get addressed (#8264) or we'll have nested workspaces (#5042) or we'll change the defaults for everyone (#1774, #6577) or packages will be able to express their preferred settings (#7940).

@ehuss ehuss closed this Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants