Simplify the cache_on_disk_if modifier to just cache_on_disk rust-lang/rust#154576
@bors try @rust-timer queue
View on GitHub
Awaiting bors try build completion.
@rustbot label: +S-waiting-on-perf
View on GitHub
⌛ Trying commit cc70ec0 with merge 72527f8…
To cancel the try build, run the command @bors try cancel.
Workflow: https://github.com/rust-lang/rust/actions/runs/23739649784
Ok, that's good
Zulip topic for context: #t-compiler/query-system > Removing key_fingerprint_style and cache_on_disk_if
timeout
@bors try
View on GitHub
⌛ Trying commit cc70ec0 with merge 5a1a969…
To cancel the try build, run the command @bors try cancel.
Workflow: https://github.com/rust-lang/rust/actions/runs/23755579483
View on GitHub
Queued 5a1a969 with parent cf7da0b, future comparison URL.
There are currently 0 preceding artifacts in the queue.
It will probably take at least ~1.0 hours until the benchmark run finishes.
Finished benchmarking commit (5a1a969): comparison URL.
Overall result: ❌✅ regressions and improvements - please read the text below
Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.
Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.
@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression
Instruction count
Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
- | - | 0 |
| Regressions ❌ (secondary) |
0.8% | [0.8%, 0.8%] | 1 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
-0.2% | [-0.6%, -0.1%] | 8 |
| All ❌✅ (primary) | - | - | 0 |
Max RSS (memory usage)
Results (primary 1.4%)
A less reliable metric. May be of interest, but not used to determine the overall result above.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
1.4% | [1.4%, 1.4%] | 1 |
| Regressions ❌ (secondary) |
- | - | 0 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
- | - | 0 |
| All ❌✅ (primary) | 1.4% | [1.4%, 1.4%] | 1 |
Cycles
Results (secondary -1.6%)
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) |
2.2% | [2.2%, 2.2%] | 1 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
-5.4% | [-5.4%, -5.4%] | 1 |
| All ❌✅ (primary) | - | - | 0 |
Binary size
This benchmark run did not return any relevant results for this metric.
Bootstrap: 483.836s -> 485.128s (0.27%)
Artifact size: 394.90 MiB -> 394.80 MiB (-0.02%)
compiler/rustc_query_impl/src/query_impl.rs · outdated
| 129 | _tcx: TyCtxt<'tcx>, |
|
| 130 | _key: rustc_middle::queries::$name::Key<'tcx>, |
|
| 131 | ) -> bool { |
|
| 132 | if !cfg!($cache_on_disk) { |
Can you use #[cfg] instead of cfg!?
Alternatively, cfg_select! might make this function nicer.
The cfg_select! version ends up being a bit visually “messier”, but has the benefit of being very explicit about exhaustively listing all possible cases, which seems like an improvement. 👍
You can actually just use $cache_on_disk.
Oh that’s right, it expands to a boolean literal, so it could be used directly as a value.
We ended up going with cfg_select! to make the various cases more explicit, but that’s good to keep in mind.
I think the cfg_select! is good because it makes clear that this is compile-time selection of code fragments, and there is no relying on the optimizer to trim dead paths.
The issue I was originally trying to work around was that #[cfg(..)] can be very clunky and verbose when trying to select an expression instead of a statement.
But cfg_select! works wonderfully in that situation.
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.
@rustbot ready
Ah, I remembered that the dev-guide will need updating too.
Since this is rollup=never and nowhere near the top of the queue, I'll push a fix.
The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes.
076dc9b06ec41bd6103dda22010fef784768ba5b was pushed.
This pull request was unapproved.
View on GitHub
⌛ Testing commit 076dc9b with merge ba11b1e...
Workflow: https://github.com/rust-lang/rust/actions/runs/23799192968
What is this?
This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing e4fdb55 (parent) -> ba11b1e (this PR)
Test differences
Show 68 test diffs
68 doctest diffs were found. These are ignored, as they are noisy.
Test dashboard
Run
cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard ba11b1e3f09829428844b74829146fcf7c38d3d0 --output-dir test-dashboardAnd then open test-dashboard/index.html in your browser to see an overview of all executed tests.
Job duration changes
- dist-x86_64-apple: 1h 38m -> 2h 7m (+29.8%)
- aarch64-apple: 3h 39m -> 2h 51m (-21.8%)
- x86_64-gnu-llvm-22-1: 1h 3m -> 1h 13m (+15.6%)
- x86_64-gnu-llvm-21-2: 1h 25m -> 1h 37m (+14.6%)
- dist-aarch64-msvc: 1h 50m -> 1h 38m (-11.3%)
- i686-gnu-2: 1h 36m -> 1h 46m (+9.8%)
- i686-msvc-1: 2h 45m -> 3h 1m (+9.6%)
- aarch64-gnu-debug: 1h 9m -> 1h 15m (+9.5%)
- dist-i686-msvc: 2h 12m -> 2h 24m (+9.3%)
- x86_64-gnu-debug: 1h 57m -> 2h 8m (+8.9%)
How to interpret the job duration changes?
Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.
Finished benchmarking commit (ba11b1e): comparison URL.
Overall result: ❌✅ regressions and improvements - no action needed
@rustbot label: -perf-regression
Instruction count
Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
- | - | 0 |
| Regressions ❌ (secondary) |
0.9% | [0.9%, 0.9%] | 1 |
| Improvements ✅ (primary) |
- | - | 0 |
| Improvements ✅ (secondary) |
-0.1% | [-0.2%, -0.1%] | 4 |
| All ❌✅ (primary) | - | - | 0 |
Max RSS (memory usage)
Results (primary -0.6%, secondary -2.0%)
A less reliable metric. May be of interest, but not used to determine the overall result above.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
2.6% | [1.0%, 3.8%] | 5 |
| Regressions ❌ (secondary) |
1.5% | [1.5%, 1.5%] | 1 |
| Improvements ✅ (primary) |
-2.8% | [-4.4%, -1.0%] | 7 |
| Improvements ✅ (secondary) |
-2.5% | [-3.8%, -1.4%] | 7 |
| All ❌✅ (primary) | -0.6% | [-4.4%, 3.8%] | 12 |
Cycles
Results (secondary -5.7%)
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 |
| Improvements ✅ (secondary) |
-5.7% | [-5.7%, -5.7%] | 1 |
| All ❌✅ (primary) | - | - | 0 |
Binary size
This benchmark run did not return any relevant results for this metric.
Bootstrap: 484.743s -> 488.578s (0.79%)
Artifact size: 394.91 MiB -> 394.90 MiB (-0.00%)
View all comments
Thanks to #154304, there is now only one remaining query with a non-trivial cache-on-disk condition (
check_liveness). But prior experiments at #153441 (comment) indicate thatcheck_livenesscan also be made non-disk-caching with little or no measurable effect.This PR takes advantage of those facts to replace the
cache_on_disk_ifmodifier with a simplercache_on_diskmodifier:separate_provide_extern, only values for “local” keys are cached to disk.cache_on_disk, values are cached to disk unconditionally.r? nnethercote (or compiler)