Skip to content

Stop passing -Zincremental-verify-ich - #2510

Open
xmakro wants to merge 1 commit into
rust-lang:mainfrom
xmakro:verify-ich-incremental-only
Open

Stop passing -Zincremental-verify-ich#2510
xmakro wants to merge 1 commit into
rust-lang:mainfrom
xmakro:verify-ich-incremental-only

Conversation

@xmakro

@xmakro xmakro commented Jul 26, 2026

Copy link
Copy Markdown

rustc-perf forces -Zincremental-verify-ich so that incremental cache verification, which normally covers a random 1/32 of loaded values, does not add noise: the subset was picked by value hash, so it differed between the two builds being compared.

Since rust-lang/rust#160130 the subset is picked by key fingerprint and session count. Both are the same for the two builds in a comparison, because rustc-perf pins the rustc version and runs the same scenarios, so the verification cost is identical on both sides and no longer noise.

We also want to build on this in a follow-up to rust-lang/rust#160214, where we want to avoid loading query results that are already green. Those values would then not be available for verification at all, so for that work it is desirable that rustc-perf measures exactly what users will run.

@xmakro
xmakro force-pushed the verify-ich-incremental-only branch from 05c927f to e9df0eb Compare July 26, 2026 20:07
@xmakro
xmakro marked this pull request as ready for review July 26, 2026 20:43
@nnethercote

Copy link
Copy Markdown
Contributor

Seems ok to me, though we'll need to be careful when we update the rustc-perf version used by CI, otherwise the improvement might be attributed to a compiler change.

@Kobzol

Kobzol commented Jul 27, 2026

Copy link
Copy Markdown
Member

This shouldn't be needed if rust-lang/rust#159796 lands, right? Let's wait for that.

@xmakro
xmakro force-pushed the verify-ich-incremental-only branch from e9df0eb to 7583e30 Compare August 10, 2026 01:59
@xmakro xmakro changed the title Only pass -Zincremental-verify-ich for incremental compiles Stop passing -Zincremental-verify-ich Aug 10, 2026
@xmakro
xmakro force-pushed the verify-ich-incremental-only branch from 7583e30 to 8662234 Compare August 10, 2026 02:04
@xmakro

xmakro commented Aug 10, 2026

Copy link
Copy Markdown
Author

I updated the PR to drop the flag entirely instead of only skipping it for non-incremental compiles. This is now possible because rust-lang/rust#160130 landed.

The concrete motivation is a follow-up to rust-lang/rust#160214 that avoids loading query results that are already green. Those values would not be available for verification at all, so forcing full verification would not show the perf saving of that work.

I think it is desirable that perf CI runs the same setting that real users would run, so we measure the right thing.

@Kobzol

Kobzol commented Aug 10, 2026

Copy link
Copy Markdown
Member

So if I understand it correctly, after rust-lang/rust#160130, the verification is based on the hash of the key, not on the hash of the value, so you're saying that the keys should be the same in both the compared runs, and we thus don't have to verify everything anymore?

But aren't some of the keys based on values from the compilation session?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants