Stop passing -Zincremental-verify-ich - #2510
Conversation
05c927f to
e9df0eb
Compare
|
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. |
|
This shouldn't be needed if rust-lang/rust#159796 lands, right? Let's wait for that. |
e9df0eb to
7583e30
Compare
7583e30 to
8662234
Compare
|
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. |
|
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? |
rustc-perf forces
-Zincremental-verify-ichso 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.