Computing crate_hash from metadata encoding instead of HIR (implements #94878) by Daniel-B-Smith · Pull Request #154724 · rust-lang/rust

Daniel-B-Smith · 2026-04-02T18:15:01Z

This PR converts the crate_hash/SVH to depend on metadata instead of HIR whenever metadata is needed. A trimmed down crate_hash is kept for dylib/binary cases where metadata is not present. It is believed that the metadata will be a more sound way to track dependency changes.

Related to #94878

The metadata hash is calculated on the raw bytes before flush because hashing the elements incrementally turned out to be too expensive.

The change to the HIR hash is potentially safe even without the metadata crate_hash change. Without that change, this PR is a performance regression. I am bundling them because I believe the HIR hash change is more likely to be safe in light of the HIR hash being less load bearing on the SVH.

The dylib/binary crate_hash removes components that I believe are not relevant to those cases. Analysis:

resolutions.visibilities_for_hashing:

The comment says: "Hash visibility information since it does not appear in HIR." Visibilities only matter to external users of the crate (i.e. consumers of metadata). Without metadata, visibility differences are not observable, so this contributes nothing for incremental correctness.

debugger_visualizers:

The comment says: "that content is exported into crate metadata, so any changes to it need to be reflected in the crate hash." Without metadata, there is nothing to export, so this is purely metadata-motivated. The visualizer file path is already covered by HIR (the attribute) and the content is loaded later only if metadata is being written.

source_file_names:

These exist solely so that the crate-hash reflects remapped source paths that get embedded into metadata (the comment explicitly says "If we included the full mapping in the SVH, we could only have reproducible builds…"). The remapping itself is already captured via dep_tracking_hash, so for incremental this is redundant.

The crate hash calculation can be reverted via -Z metadata-crate-hash=no. I manually confirmed that setting that flag generates the same hash as the commit prior to my change. The test for that flag only checks that it changes the value of the hash and does not check the specific value of the hash. I could add a check for the latter, but I'm nervous that a hash golden test would be a maintenance headache.

nnethercote · 2026-04-09T23:26:03Z

@bors try @rust-timer queue

Daniel-B-Smith · 2026-07-07T02:19:31Z

I've removed the "do not submit" because I have now tested 100+ commits with tokio, ripgrep, bstr, zstd, serde_json, and parking_lot.

No feedback on Zulip on the XXH3 vs SipHash question, so I do want to flag that here. I don't see any difference in correctness between the two hashes, but XXH3 is dramatically faster.

oli-obk · 2026-07-08T17:23:47Z

Please remove the auto inserted LLM tool advertisement from the commit

Daniel-B-Smith · 2026-07-09T16:49:05Z

Removed the LLM tag from the XXH3 commit. I left it there to err on the side of disclosure, but happy to remove it.

oli-obk

Some more docs, then this seems good. Since we're cutting a beta today, if we land it in the next few days we'll have lots of time to test it out in practice and due to the flag, trivially revert it if there are issues

View changes since this review

oli-obk · 2026-08-14T18:44:46Z

+            .local_crate_hash
+            .get()
+            .expect("crate_hash(LOCAL_CRATE) called before metadata encoding")
+    } else {


Leave a comment to that regard

on the first arm, stating that we don't need to compute it because we already did so for metadata encoding

on the second arm that were doing the same HIR based crate hash computation that metadata encoding is doing

rustbot · 2026-08-14T19:09:33Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

rustbot · 2026-08-16T22:00:22Z

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

rustc-dev-guide is developed in its own repository. If possible, consider making this change to rust-lang/rustc-dev-guide instead.

cc @BoxyUwU, @tshepang

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

The SVH is computed by hashing the entire encoded metadata blob, and SipHash-128 (~2.5 Ir/byte) dominated the cost — it accounted for the whole of the measured regression on metadata-heavy crates. Switch the byte pass to XXH3-128, which is ~10x cheaper per byte on its runtime-detected AVX2 path. XXH3 is non-cryptographic, which is fine here: the SVH is only a change detector, must be reproducible (so it is never keyed secretly, meaning even the previous SipHash use was not collision-resistant), and compiling a crate already executes its build scripts and proc-macros, so the crate author is trusted regardless. The untracked supplements (dep-tracking hash and the HIR hash for e.g. DefKind::GlobalAsm) are still folded in via a StableHasher fingerprint, so coverage is unchanged. twox-hash 2.x is already in the dependency graph via ruzstd; this only adds a direct edge (xxhash3_128 + std features for the runtime SIMD dispatch).

rustbot · 2026-08-16T22:09:41Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Daniel-B-Smith · 2026-08-17T01:07:56Z

@rustbot ready

Daniel-B-Smith · 2026-08-17T01:08:20Z

the LLVM thing was a bad rebase that has been fixed. I've also double checked that the disable flag still works.

oli-obk · 2026-08-17T06:20:53Z

@bors r+ rollup=never

rust-bors · 2026-08-17T06:20:56Z

📌 Commit e7d9d02 has been approved by oli-obk

It is now in the queue for this repository.

rust-bors · 2026-08-17T08:58:14Z

⌛ Testing commit e7d9d02 with merge 1ab92c7...

Workflow: https://github.com/rust-lang/rust/actions/runs/32012882448

bjorn3 · 2026-08-17T09:17:26Z

 #[derive(MetadataEncodable, BlobDecodable)]
 pub(crate) struct CrateHeader {
    pub(crate) triple: TargetTuple,
-    pub(crate) hash: Svh,


Um, I'm pretty sure removing this field will reintroduce the problem of #76720. Putting the crate hash after the unhashed data does not work as that makes the offset of the crate hash differ between rustc versions.

View changes since the review

That issue was fixed by disabling incremental across stages #111982. That is still the case in main:

rust/src/bootstrap/src/core/builder/cargo.rs

Line 1290 in 6d656b1

if self.config.incremental && compiler.stage == 0 {

.

Even if that weren't the case, I'm pretty sure this would be fine because the version mismatch will error out before anything can try to decode the crate hash. That check is here:

rust/compiler/rustc_metadata/src/locator.rs

Line 908 in 6d656b1

match blob.check_compatibility(cfg_version) {

. The hash decoding is now lazy, so that check will fail before any caller can call get_crate_hash().

Just to be sure, I followed the reproduction instructions and everything worked fine. Specifically:

git checkout 34baba5394fcbda4cba7b7c19 # Base for my branch ./x clean ./x build git checkout e7d9d02a6311e5cdf09dbcee73ac5 # My branch at the time of this comment ./x build ./x test

Everything succeeded.

That issue was fixed by disabling incremental across stages #111982.

According to the PR description that PR fixes the problem when combined with #111329 which this PR breaks. I don't know if #111329 is actually necessary though.

I'm pretty sure this would be fine because the version mismatch will error out before anything can try to decode the crate hash.

The version stays constant when you make local modifications.

Just to be sure, I followed the reproduction instructions and everything worked fine.

According to #111329 (comment) both a change to the metadata format (without bumping METADATA_VERSION like this PR does rather than keeping it the same like everything that doesn't touch the metadata header does) and a dependency version bump have to be made at the same time to trigger the bug.

That said, I just noticed that when I said

Putting the crate hash after the unhashed data does not work as that makes the offset of the crate hash differ between rustc versions.

this was incorrect. I misread the UNHASHED_POS_* as being the CrateRootUnhashed rather than just a u64 indicating the position of it. A u64 is fine, so I'm retracting my r-. My apologies for misreading the code.

bjorn3 · 2026-08-17T09:29:35Z

@bors r-

rust-bors · 2026-08-17T09:29:40Z

This pull request was unapproved.

Auto build was cancelled due to unapproval. Cancelled workflows:

https://github.com/rust-lang/rust/actions/runs/32012882448

View changes since this unapproval

Daniel-B-Smith · 2026-08-17T17:43:59Z

@rustbot ready

rustbot · 2026-08-17T17:44:04Z

Error: shortcut handler unexpectedly failed in this comment: failed to add labels

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #triagebot on Zulip.

bjorn3 · 2026-08-17T18:40:18Z

+#[derive(MetadataEncodable, LazyDecodable)]
+pub(crate) struct CrateRootUnhashed {
+    extra_filename: String,
+}


Why is this actually unhashed? This is stored in the hashed part of the metadata of dependent crates, right?

View changes since the review

It is not hashed currently. #86045 removed this from the hash.

If this field is included in the hash, https://github.com/rust-lang/rust/blob/6d656b1efca82491a110b476ac9bbe712628ccb6/tests/run-make/emit-path-unhashed/rmake.rs fails specifically.

Was the extra_filename for dependencies previously covered by the crate hash? It almost certainly is with this PR.

Good point. It certainly was not. I've moved that field to the unhashed portion and added a test.

I put that into a separate commit for review, but it's probably worth squashing before merging.

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 2, 2026