Skip to content

[WIP] feat: maximize artifact reuse between different modes #15627

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

weihanglo
Copy link
Member

What does this PR try to resolve?

As of this writing, it aims to reuse rmeta between build and check modes.
(one emits rmeta + rlib, and the other emits rmeta)

This has a caveat that DefId in rmeta files might not match between rustc
invocations with different --emits values, and that might lead to rustc
rejecting the input rmeta. To avoid failures in rustc, this is currently
guarded by fingerprint to ensure all output files present, if not, rerun.
This make running check after build safe: build's rmeta files might
have more information and rustc is correct on rejection.

How to test and review this PR?

Proof of concept. NOT READY FOR REVIEW

@rustbot rustbot added A-layout Area: target output directory layout, naming, and organization A-rebuild-detection Area: rebuild detection and fingerprinting labels Jun 4, 2025
@weihanglo weihanglo changed the title feat: maximizes artifact reuse between different modes [WIP] feat: maximize artifact reuse between different modes Jun 4, 2025
@@ -404,7 +404,6 @@ fn collision_doc_profile_split() {
p.cargo("doc")
.with_stderr_data(
str![[r#"
[CHECKING] common v1.0.0
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

proc-macro build has generated rmeta first, so skipping checking.

@@ -2416,7 +2416,7 @@ fn doc_lib_true() {

// Verify that it emits rmeta for the bin and lib dependency.
assert_eq!(p.glob("target/debug/artifact/*.rlib").count(), 0);
assert_eq!(p.glob("target/debug/deps/libbar-*.rmeta").count(), 2);
assert_eq!(p.glob("target/debug/deps/libbar-*.rmeta").count(), 1);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Artifact lib build generated rmeta first, so doc build can skip checking dependencies.

As of this writing, it aims to reuse `rmeta` between build and check modes.
(one emits rmeta + rlib, and the other emits rmeta)

This has a caveat that `DefId` in rmeta files might not match between rustc
invocations with different `--emits` values, and that might lead to rustc
rejecting the input rmeta. To avoid failures in rustc, this is currently
guarded by fingerprint to ensure all output files present, if not, rerun.
This make running `check` after `build` safe: `build`'s rmeta files might
have more information and rustc is correct on rejection.
@epage
Copy link
Contributor

epage commented Jun 4, 2025

This make running check after build safe: build's rmeta files might
have more information and rustc is correct on rejection.

Is this meant to be incremental to eventually allow cargo check && cargo build? Otherwise, cargo build && cargo check seems rare enough to not justify this.

@weihanglo
Copy link
Member Author

Is this meant to be incremental to eventually allow cargo check && cargo build?

I would love to see it happen. Still experimenting with rustc.

Otherwise, cargo build && cargo check seems rare enough to not justify this.

You may still run cargo build && cargo doc which the latter would benefit.

@weihanglo
Copy link
Member Author

…to eventually allow cargo check && cargo build?

rust-lang/compiler-team#738 is partially related

@celinval
Copy link

celinval commented Jun 5, 2025

This make running check after build safe: build's rmeta files might
have more information and rustc is correct on rejection.

Is this meant to be incremental to eventually allow cargo check && cargo build? Otherwise, cargo build && cargo check seems rare enough to not justify this.

Allowing cargo check && cargo build to reuse artifacts will require changes to the compiler, and it could potentially slow down cargo check (as @bjorn3 explained here), so it's not clear at this point if it is something feasible or desirable. I can see that fitting in with selective compilation, which is also highly experimental.

That said, I think a lot of people run cargo clippy before commit or in CI, and they would benefit from cargo build && cargo check not requiring 2 completely separate build cycles.

What is the cost / risks of going through this change though?

@bjorn3
Copy link
Member

bjorn3 commented Jun 5, 2025

A check and regular build will produce a different crate hash, so if you do cargo check; cargo build -p some_dep; cargo check you are now forced to rebuild everything that depends on some_dep as the crate hash of some_dep changed.

@celinval
Copy link

celinval commented Jun 5, 2025

Hummm.. is that because of the -p?

@bjorn3
Copy link
Member

bjorn3 commented Jun 5, 2025

The is used -p to only rebuild part of the crates in build mode. The same would apply if you run cargo build from inside the working directory of a single crate in the workspace or if you interrupt the build halfway through.

@celinval
Copy link

celinval commented Jun 5, 2025

would the same happen if you run cargo check from inside the working directory of a single crate in the workspace?

@weihanglo
Copy link
Member Author

weihanglo commented Jun 5, 2025

A check and regular build will produce a different crate hash, so if you do cargo check; cargo build -p some_dep; cargo check you are now forced to rebuild everything that depends on some_dep as the crate hash of some_dep changed.

This is a good point. By sharing it might make rust-analyzer a bit worse to work with cargo.

I think the real world situation is more complicated for partial build, as other factors like feature unification may kick in. I just tried cargo +stable build -p rusqlite first and then do a full cargo +stanlf build in Cargo, smallvec is used but not libc. So it is not only an issue of crate hash difference. By the way, can you clarify a bit which “crate hash” you are referring to?

@bjorn3
Copy link
Member

bjorn3 commented Jun 6, 2025

By the way, can you clarify a bit which “crate hash” you are referring to?

The crate hash field in the crate metadata. This field is also recorded in the crate metadata for all dependencies. If it changes, then next time rustc has to load it as indirect dependency, it will reject the .rlib/.rmeta file as having the wrong crate hash, thus forcing all dependents of a crate to be recompiled whenever the crate hash changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-layout Area: target output directory layout, naming, and organization A-rebuild-detection Area: rebuild detection and fingerprinting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants