Uh oh!
There was an error while loading. Please reload this page.
Improve the forcing/promotion functions in DepKindVTable - #153122
Conversation
Zalathar
commented
Feb 26, 2026
This is not fully-baked yet, but I want to run PR CI and perf to check for unanticipated problems. @bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Improve the forcing/promotion functions in `DepKindVTable`
Uh oh!
There was an error while loading. Please reload this page.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
rust-timer
commented
Feb 26, 2026
Finished benchmarking commit (dff6afa): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking 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. @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -3.7%, secondary 4.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -3.1%, secondary -8.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 492.161s -> 478.802s (-2.71%) |
2aba5f0 to
3b6451eCompareZalathar
commented
Feb 27, 2026
r? nnethercote (or compiler) |
nnethercote
commented
Feb 27, 2026
Fantastic perf results! I would like to eliminate @bors r+ |
Zalathar
commented
Feb 27, 2026
Right now, So I currently don't see a way to usefully put a query-vtable pointer into |
| /// True if a key can _potentially_ be recovered from a key fingerprint | ||
| /// with this style. | ||
| /// | ||
| /// For some key types, recovery is possible but not guaranteed. |
There was a problem hiding this comment.
This seems a bit misleading. Do we have any types where recovery will fail?
There was a problem hiding this comment.
Yes, it's possible for DefId to fail recovery, for example.
I verified this by adding a panic to the failure path of one of the try_recover_key calls, and then running x test incremental.
This comment has been minimized.
This comment has been minimized.
- `force_from_dep_node` → `force_from_dep_node_fn` - `try_load_from_disk_cache` → `promote_from_disk_fn` This commit also inlines the wrapper function around `promote_from_disk_fn`.
rustbot
commented
Mar 1, 2026
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. |
Zalathar
commented
Mar 1, 2026
Rebased to fix trivial conflict. @bors r=nnethercote |
This comment has been minimized.
This comment has been minimized.
Uh oh!
There was an error while loading. Please reload this page.
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 024757f (parent) -> 28b5c1c (this PR) Test differencesShow 4 test diffs4 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 28b5c1cc084d3d21168f034b39c23193a9a7f127 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
rust-timer
commented
Mar 1, 2026
Finished benchmarking commit (28b5c1c): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary -2.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -3.0%, secondary -6.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 477.708s -> 479.821s (0.44%) |
This is a bundle of changes to the two function pointers in
DepKindVTablethat are responsible for forcing dep nodes, or promoting disk-cached values from the previous session into memory.The perf improvements to incr-unchanged and incr-patched are likely from skipping more of the “promotion” plumbing for queries that never cache to disk.