Skip to content

Select cache values to verify by key fingerprint, not value fingerprint - #160130

Open
xmakro wants to merge 1 commit into
rust-lang:mainfrom
xmakro:cleanup/deterministic-verify-sampling
Open

Select cache values to verify by key fingerprint, not value fingerprint#160130
xmakro wants to merge 1 commit into
rust-lang:mainfrom
xmakro:cleanup/deterministic-verify-sampling

Conversation

@xmakro

@xmakro xmakro commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

rustc verifies ~1/32 of values loaded from the incremental cache by re-hashing them. The subset is selected by the value fingerprint, so it changes whenever a value changes and between any two compiler builds. That makes verification failures flaky to reproduce, and it moves the re-hashing cost around between the two builds rustc-perf compares, which is why rustc-perf forces -Zincremental-verify-ich on every benchmarked invocation.

This PR selects by the key fingerprint and the session count instead. Both are fixed for a given previous session, so re-running a failed build verifies the same nodes and a verification failure reproduces on retry. to_smaller_hash mixes both fingerprint halves because neither half is evenly distributed on its own: DefPathHash keys share the StableCrateId, HirId keys carry a sequential id.

The per-session sampling rate is unchanged at 1/32, and -Zincremental-verify-ich is unchanged. Coverage over time improves: the subset rotates with the session count and sweeps the entire cache every 32 sessions, while under value selection a node whose value never changed kept its roll forever, so 31/32 of unchanging values were never verified on a given toolchain.

Between two compiler builds the subset still differs, because key fingerprints incorporate the rustc version through StableCrateId. Pinning the version via RUSTC_FORCE_RUSTC_VERSION and RUSTC_OVERRIDE_VERSION_STRING makes corresponding sessions identical; session counts align because both builds run the same scenario sequence from a fresh incremental directory. That would let rustc-perf stop forcing full verification and measure the incremental configuration users actually run.

Coverage across compiler releases is unchanged: the version salt reshuffles the subset each release under either scheme, and compiletest forces full verification in all incremental tests.

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) 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 Jul 29, 2026
@xmakro

xmakro commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

r? @cjgillot
cc @nnethercote

@xmakro
xmakro marked this pull request as ready for review July 29, 2026 11:26
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 29, 2026
@cjgillot

cjgillot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

That would let rustc-perf stop forcing full verification and measure the incremental configuration users actually run.

I'm not sure this is a goal. Having rustc-perf force fingerprinting gives an upper bound and may detect bottlenecks that we would otherwise miss.

@bors try @rust-timer queue

r=me if perf is green

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 4, 2026
@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Aug 4, 2026
… r=<try>

Select cache values to verify by key fingerprint, not value fingerprint
@rust-bors

rust-bors Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: db3d63f (db3d63fe29bc48543b30f9ba77be28760a8f96c7)
Base parent: 0b63def (0b63defc8fd68957c635fe61f097b84df8fd9611)

@rust-timer

This comment has been minimized.

@xmakro

xmakro commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

The motivation is a planned follow-up to #160214 that carries unchanged query cache values forward as raw bytes, without decoding them. That changes two things:

  1. Recomputation no longer re-hashes those values, so sampled verification is the only check their bytes ever get. Value selection pins a value's roll forever, so 31/32 of them would never be checked. Session rotation covers everything within 32 sessions.

  2. Forcing full verification makes rustc-perf decode and re-hash every carried value, which is exactly the work carry removes, so the benchmark would show near zero for a real win in the configuration users run. Deterministic selection removes the noise that forced the flag in the first place.

Keeping a forced-verification run as an upper bound seems fine. I just want a sampling scheme that a user-matching configuration could rely on.

@xmakro

xmakro commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

That would let rustc-perf stop forcing full verification and measure the incremental configuration users actually run.

I'm not sure this is a goal. Having rustc-perf force fingerprinting gives an upper bound and may detect bottlenecks that we would otherwise miss.

@bors try @rust-timer queue

r=me if perf is green

A motivation would be for a follow-up to #160214 where we carry unchanged query cache values forward as raw bytes, without decoding them. Forcing full verification makes rustc-perf decode and re-hash every carried value, which is the work carry removes, so the benchmark would show no win. In this case, it would be good if rustc-perf resembled what users are running.

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (db3d63f): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (primary -0.2%, secondary -0.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
0.4% [0.4%, 0.4%] 1
Regressions ❌
(secondary)
0.4% [0.4%, 0.4%] 1
Improvements ✅
(primary)
-0.9% [-0.9%, -0.9%] 1
Improvements ✅
(secondary)
-0.7% [-0.9%, -0.6%] 2
All ❌✅ (primary) -0.2% [-0.9%, 0.4%] 2

Cycles

Results (primary -0.1%, secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
0.6% [0.4%, 0.9%] 6
Regressions ❌
(secondary)
0.6% [0.4%, 1.1%] 6
Improvements ✅
(primary)
-0.8% [-1.2%, -0.6%] 6
Improvements ✅
(secondary)
-0.7% [-0.8%, -0.6%] 4
All ❌✅ (primary) -0.1% [-1.2%, 0.9%] 12

Binary size

Results (primary -0.1%, secondary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.1% [-0.1%, -0.0%] 74
Improvements ✅
(secondary)
-0.1% [-0.1%, -0.0%] 57
All ❌✅ (primary) -0.1% [-0.1%, -0.0%] 74

Bootstrap: 489.577s -> 488.977s (-0.12%)
Artifact size: 390.24 MiB -> 390.21 MiB (-0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 4, 2026
@cjgillot

cjgillot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

@bors r+

@rust-bors

rust-bors Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 012c356 has been approved by cjgillot

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 10. This pull request will be tested once the tree is reopened.

Reason for tree closure: manually handling queue due to backlog

@rust-bors rust-bors Bot 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 Aug 4, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Aug 5, 2026
…y-sampling, r=cjgillot

Select cache values to verify by key fingerprint, not value fingerprint

rustc verifies ~1/32 of values loaded from the incremental cache by re-hashing them. The subset is selected by the value fingerprint, so it changes whenever a value changes and between any two compiler builds. That makes verification failures flaky to reproduce, and it moves the re-hashing cost around between the two builds rustc-perf compares, which is why rustc-perf forces `-Zincremental-verify-ich` on every benchmarked invocation.

This PR selects by the key fingerprint and the session count instead. Both are fixed for a given previous session, so re-running a failed build verifies the same nodes and a verification failure reproduces on retry. `to_smaller_hash` mixes both fingerprint halves because neither half is evenly distributed on its own: `DefPathHash` keys share the `StableCrateId`, `HirId` keys carry a sequential id.

The per-session sampling rate is unchanged at 1/32, and `-Zincremental-verify-ich` is unchanged. Coverage over time improves: the subset rotates with the session count and sweeps the entire cache every 32 sessions, while under value selection a node whose value never changed kept its roll forever, so 31/32 of unchanging values were never verified on a given toolchain.

Between two compiler builds the subset still differs, because key fingerprints incorporate the rustc version through `StableCrateId`. Pinning the version via `RUSTC_FORCE_RUSTC_VERSION` and `RUSTC_OVERRIDE_VERSION_STRING` makes corresponding sessions identical; session counts align because both builds run the same scenario sequence from a fresh incremental directory. That would let rustc-perf stop forcing full verification and measure the incremental configuration users actually run.

Coverage across compiler releases is unchanged: the version salt reshuffles the subset each release under either scheme, and compiletest forces full verification in all incremental tests.
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 5, 2026
…y-sampling, r=cjgillot

Select cache values to verify by key fingerprint, not value fingerprint

rustc verifies ~1/32 of values loaded from the incremental cache by re-hashing them. The subset is selected by the value fingerprint, so it changes whenever a value changes and between any two compiler builds. That makes verification failures flaky to reproduce, and it moves the re-hashing cost around between the two builds rustc-perf compares, which is why rustc-perf forces `-Zincremental-verify-ich` on every benchmarked invocation.

This PR selects by the key fingerprint and the session count instead. Both are fixed for a given previous session, so re-running a failed build verifies the same nodes and a verification failure reproduces on retry. `to_smaller_hash` mixes both fingerprint halves because neither half is evenly distributed on its own: `DefPathHash` keys share the `StableCrateId`, `HirId` keys carry a sequential id.

The per-session sampling rate is unchanged at 1/32, and `-Zincremental-verify-ich` is unchanged. Coverage over time improves: the subset rotates with the session count and sweeps the entire cache every 32 sessions, while under value selection a node whose value never changed kept its roll forever, so 31/32 of unchanging values were never verified on a given toolchain.

Between two compiler builds the subset still differs, because key fingerprints incorporate the rustc version through `StableCrateId`. Pinning the version via `RUSTC_FORCE_RUSTC_VERSION` and `RUSTC_OVERRIDE_VERSION_STRING` makes corresponding sessions identical; session counts align because both builds run the same scenario sequence from a fresh incremental directory. That would let rustc-perf stop forcing full verification and measure the incremental configuration users actually run.

Coverage across compiler releases is unchanged: the version salt reshuffles the subset each release under either scheme, and compiletest forces full verification in all incremental tests.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Aug 5, 2026
…y-sampling, r=cjgillot

Select cache values to verify by key fingerprint, not value fingerprint

rustc verifies ~1/32 of values loaded from the incremental cache by re-hashing them. The subset is selected by the value fingerprint, so it changes whenever a value changes and between any two compiler builds. That makes verification failures flaky to reproduce, and it moves the re-hashing cost around between the two builds rustc-perf compares, which is why rustc-perf forces `-Zincremental-verify-ich` on every benchmarked invocation.

This PR selects by the key fingerprint and the session count instead. Both are fixed for a given previous session, so re-running a failed build verifies the same nodes and a verification failure reproduces on retry. `to_smaller_hash` mixes both fingerprint halves because neither half is evenly distributed on its own: `DefPathHash` keys share the `StableCrateId`, `HirId` keys carry a sequential id.

The per-session sampling rate is unchanged at 1/32, and `-Zincremental-verify-ich` is unchanged. Coverage over time improves: the subset rotates with the session count and sweeps the entire cache every 32 sessions, while under value selection a node whose value never changed kept its roll forever, so 31/32 of unchanging values were never verified on a given toolchain.

Between two compiler builds the subset still differs, because key fingerprints incorporate the rustc version through `StableCrateId`. Pinning the version via `RUSTC_FORCE_RUSTC_VERSION` and `RUSTC_OVERRIDE_VERSION_STRING` makes corresponding sessions identical; session counts align because both builds run the same scenario sequence from a fresh incremental directory. That would let rustc-perf stop forcing full verification and measure the incremental configuration users actually run.

Coverage across compiler releases is unchanged: the version salt reshuffles the subset each release under either scheme, and compiletest forces full verification in all incremental tests.
rust-bors Bot pushed a commit that referenced this pull request Aug 5, 2026
…uwer

Rollup of 22 pull requests

Successful merges:

 - #158147 (std: fix stack buffer overflow in Windows junction_point)
 - #159784 (Hint that memchr returns an in-bounds index)
 - #160130 (Select cache values to verify by key fingerprint, not value fingerprint)
 - #160343 (Rename `OutlivesPredicate` to `OutlivesClause`)
 - #160360 (Remove rustc_middle dependency on rustc_hir_pretty)
 - #160387 (rustc_codegen_ssa: Correctly apply the static `--jobs-backend` limit to backend parallelism)
 - #160422 (move mir-opt miri tests to CI logic)
 - #160444 (Avoid resolving path keywords outside `TypeNS`)
 - #160510 (Resolver: (un)tracked borrows for `CmRefCell` made safe my unsafe speculative flag)
 - #155424 ([blocked] Link to proposed LLM policy in CONTRIBUTING and pull request template)
 - #158726 (std: move futex implementations into sys::sync::futex)
 - #159225 (Split IncrCompSession out of Session)
 - #159820 (Make the `rustc_unsafe_specialization_marker` attribute actually `unsafe`)
 - #160198 (Rework `smallest_range_containing` to handle duplicates)
 - #160362 (Split `SpannedTypeVisitor` into its own crate, `rustc_ty_walk`)
 - #160390 (autodiff: Handle slice-tailed DSTs in type trees)
 - #160420 (Suggest `cast_signed()` for overflowing signed integer literals)
 - #160520 (Add some tests for specialization)
 - #160522 (fix(bootstrap): Normalize the names of proc macro dependency crates)
 - #160531 (docs: fix typo in AllowExprMetavar comment)
 - #160538 (Update expect messages in tcp.rs doc examples to follow the style guide)
 - #160548 (bootstrap: Register `coverage-map` and `coverage-run` aliases via a separate step)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Aug 5, 2026
…y-sampling, r=cjgillot

Select cache values to verify by key fingerprint, not value fingerprint

rustc verifies ~1/32 of values loaded from the incremental cache by re-hashing them. The subset is selected by the value fingerprint, so it changes whenever a value changes and between any two compiler builds. That makes verification failures flaky to reproduce, and it moves the re-hashing cost around between the two builds rustc-perf compares, which is why rustc-perf forces `-Zincremental-verify-ich` on every benchmarked invocation.

This PR selects by the key fingerprint and the session count instead. Both are fixed for a given previous session, so re-running a failed build verifies the same nodes and a verification failure reproduces on retry. `to_smaller_hash` mixes both fingerprint halves because neither half is evenly distributed on its own: `DefPathHash` keys share the `StableCrateId`, `HirId` keys carry a sequential id.

The per-session sampling rate is unchanged at 1/32, and `-Zincremental-verify-ich` is unchanged. Coverage over time improves: the subset rotates with the session count and sweeps the entire cache every 32 sessions, while under value selection a node whose value never changed kept its roll forever, so 31/32 of unchanging values were never verified on a given toolchain.

Between two compiler builds the subset still differs, because key fingerprints incorporate the rustc version through `StableCrateId`. Pinning the version via `RUSTC_FORCE_RUSTC_VERSION` and `RUSTC_OVERRIDE_VERSION_STRING` makes corresponding sessions identical; session counts align because both builds run the same scenario sequence from a fresh incremental directory. That would let rustc-perf stop forcing full verification and measure the incremental configuration users actually run.

Coverage across compiler releases is unchanged: the version salt reshuffles the subset each release under either scheme, and compiletest forces full verification in all incremental tests.
rust-bors Bot pushed a commit that referenced this pull request Aug 5, 2026
…uwer

Rollup of 25 pull requests

Successful merges:

 - #154585 (treat no_mangle_generic_items as hard error instead of lint warning)
 - #158147 (std: fix stack buffer overflow in Windows junction_point)
 - #160130 (Select cache values to verify by key fingerprint, not value fingerprint)
 - #160343 (Rename `OutlivesPredicate` to `OutlivesClause`)
 - #160360 (Remove rustc_middle dependency on rustc_hir_pretty)
 - #160387 (rustc_codegen_ssa: Correctly apply the static `--jobs-backend` limit to backend parallelism)
 - #160422 (move mir-opt miri tests to CI logic)
 - #160444 (Avoid resolving path keywords outside `TypeNS`)
 - #160510 (Resolver: (un)tracked borrows for `CmRefCell` made safe my unsafe speculative flag)
 - #155424 ([blocked] Link to proposed LLM policy in CONTRIBUTING and pull request template)
 - #158726 (std: move futex implementations into sys::sync::futex)
 - #158762 (Emit thumb code on VEX V5)
 - #159225 (Split IncrCompSession out of Session)
 - #159820 (Make the `rustc_unsafe_specialization_marker` attribute actually `unsafe`)
 - #160198 (Rework `smallest_range_containing` to handle duplicates)
 - #160362 (Split `SpannedTypeVisitor` into its own crate, `rustc_ty_walk`)
 - #160390 (autodiff: Handle slice-tailed DSTs in type trees)
 - #160420 (Suggest `cast_signed()` for overflowing signed integer literals)
 - #160516 (Add regression test for HRTB projection in closure)
 - #160520 (Add some tests for specialization)
 - #160522 (fix(bootstrap): Normalize the names of proc macro dependency crates)
 - #160523 (Add regression test for opaque type)
 - #160531 (docs: fix typo in AllowExprMetavar comment)
 - #160538 (Update expect messages in tcp.rs doc examples to follow the style guide)
 - #160548 (bootstrap: Register `coverage-map` and `coverage-run` aliases via a separate step)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Aug 5, 2026
…y-sampling, r=cjgillot

Select cache values to verify by key fingerprint, not value fingerprint

rustc verifies ~1/32 of values loaded from the incremental cache by re-hashing them. The subset is selected by the value fingerprint, so it changes whenever a value changes and between any two compiler builds. That makes verification failures flaky to reproduce, and it moves the re-hashing cost around between the two builds rustc-perf compares, which is why rustc-perf forces `-Zincremental-verify-ich` on every benchmarked invocation.

This PR selects by the key fingerprint and the session count instead. Both are fixed for a given previous session, so re-running a failed build verifies the same nodes and a verification failure reproduces on retry. `to_smaller_hash` mixes both fingerprint halves because neither half is evenly distributed on its own: `DefPathHash` keys share the `StableCrateId`, `HirId` keys carry a sequential id.

The per-session sampling rate is unchanged at 1/32, and `-Zincremental-verify-ich` is unchanged. Coverage over time improves: the subset rotates with the session count and sweeps the entire cache every 32 sessions, while under value selection a node whose value never changed kept its roll forever, so 31/32 of unchanging values were never verified on a given toolchain.

Between two compiler builds the subset still differs, because key fingerprints incorporate the rustc version through `StableCrateId`. Pinning the version via `RUSTC_FORCE_RUSTC_VERSION` and `RUSTC_OVERRIDE_VERSION_STRING` makes corresponding sessions identical; session counts align because both builds run the same scenario sequence from a fresh incremental directory. That would let rustc-perf stop forcing full verification and measure the incremental configuration users actually run.

Coverage across compiler releases is unchanged: the version salt reshuffles the subset each release under either scheme, and compiletest forces full verification in all incremental tests.
rust-bors Bot pushed a commit that referenced this pull request Aug 5, 2026
…uwer

Rollup of 25 pull requests

Successful merges:

 - #158147 (std: fix stack buffer overflow in Windows junction_point)
 - #160130 (Select cache values to verify by key fingerprint, not value fingerprint)
 - #160343 (Rename `OutlivesPredicate` to `OutlivesClause`)
 - #160360 (Remove rustc_middle dependency on rustc_hir_pretty)
 - #160387 (rustc_codegen_ssa: Correctly apply the static `--jobs-backend` limit to backend parallelism)
 - #160422 (move mir-opt miri tests to CI logic)
 - #160444 (Avoid resolving path keywords outside `TypeNS`)
 - #160510 (Resolver: (un)tracked borrows for `CmRefCell` made safe by unsafe speculative flag)
 - #155424 ([blocked] Link to proposed LLM policy in CONTRIBUTING and pull request template)
 - #158726 (std: move futex implementations into sys::sync::futex)
 - #158762 (Emit thumb code on VEX V5)
 - #159225 (Split IncrCompSession out of Session)
 - #159820 (Make the `rustc_unsafe_specialization_marker` attribute actually `unsafe`)
 - #160198 (Rework `smallest_range_containing` to handle duplicates)
 - #160362 (Split `SpannedTypeVisitor` into its own crate, `rustc_ty_walk`)
 - #160390 (autodiff: Handle slice-tailed DSTs in type trees)
 - #160420 (Suggest `cast_signed()` for overflowing signed integer literals)
 - #160501 (Add bootstrap CLI snapshot test for testing miri)
 - #160516 (Add regression test for HRTB projection in closure)
 - #160520 (Add some tests for specialization)
 - #160522 (fix(bootstrap): Normalize the names of proc macro dependency crates)
 - #160523 (Add regression test for opaque type)
 - #160531 (docs: fix typo in AllowExprMetavar comment)
 - #160538 (Update expect messages in tcp.rs doc examples to follow the style guide)
 - #160548 (bootstrap: Register `coverage-map` and `coverage-run` aliases via a separate step)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants