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

Downstream dependencies of a crate are rebuilt despite the changes not being public-facing #14604

Open
osiewicz opened this issue Sep 26, 2024 · 2 comments
Labels
A-build-execution Area: anything dealing with executing the compiler A-rebuild-detection Area: rebuild detection and fingerprinting C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-build S-blocked-external Status: ❌ blocked on something out of the direct control of the Cargo project, e.g., upstream fix

Comments

@osiewicz
Copy link
Contributor

osiewicz commented Sep 26, 2024

Problem

Over at Zed we've noticed that even seemingly innocent changes to the crate that has many dependents causes us to rebuild (almost) the whole world. Adding a dbg! statement to the body of non-inlineable function, formatting code, updating a dependency to a new minor version and such still forces us to rebuild all dependants of an affected crate.
Our build graph could probably be improved by splitting out crates into smaller units, but the underlying issue is that we're rebuilding the world even when the public API of a crate did not change.
Attached is our timing file for making non-public-facing change to one of the core crates of Zed: timings.zip
What could take about 9 seconds (rebuilding project + relinking zed) takes 18 seconds instead.

Proposed Solution

There are efforts in flight to make Cargo rely less on mtime as a trigger for rebuilding the crate, but we are interested in an even stronger form of this PR where only a change to the public API of a crate should trigger a rebuild of the downstream dependants.

Notes

We have an internal prototype for one potential solution that relies on rustc emitting a hash of the public API of a crate as a part of it's "Metadata Finished" notification (Zulip Thread, rustc and cargo).

@osiewicz osiewicz added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage. labels Sep 26, 2024
@epage epage added A-build-execution Area: anything dealing with executing the compiler Command-build A-rebuild-detection Area: rebuild detection and fingerprinting labels Sep 26, 2024
@epage
Copy link
Contributor

epage commented Sep 26, 2024

There is a strong precedence to do this within the C++ world where they have separate "definitions" (.h) from "source" (.cpp) and a lot of effort is put into keeping as much out of "definition" files as possible, including incurring extra runtime overhead through the pimpl idiom.

This will be dependent on rustc being able to provide us a hash of the API which I expect will contain most of the risk for this.

Once we have this, generics will be an easy way for people to shoot themselves in the foot wrt rebuild time. There has been talk about the compile automatically un-inlining a non-generic section of the function. I wonder if a good first step in that direction would be a clippy lint to help people find where they could do that transformation manually to maximize the benefit they'd get from this feature.

There are efforts in flight to make Cargo rely less on #14137, but we are interested in an even stronger form of this PR where only a change to the public API of a crate should trigger a rebuild of the downstream dependants.

btw I believe these are independent. One is about how we fingerprint your source while the other is about how we fingerprint your dependencies.

@epage epage added S-blocked-external Status: ❌ blocked on something out of the direct control of the Cargo project, e.g., upstream fix and removed S-triage Status: This issue is waiting on initial triage. labels Sep 26, 2024
@epage
Copy link
Contributor

epage commented Sep 26, 2024

Marked this as blocked due to the need for compiler support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-execution Area: anything dealing with executing the compiler A-rebuild-detection Area: rebuild detection and fingerprinting C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-build S-blocked-external Status: ❌ blocked on something out of the direct control of the Cargo project, e.g., upstream fix
Projects
None yet
Development

No branches or pull requests

2 participants