Uh oh!
There was an error while loading. Please reload this page.
perf: Cache already-checked types in the privacy visitor - #160317
Conversation
petrochenkov
commented
Aug 1, 2026
Something similar was tried in #147486, but the caching overhead was found to be too large in most cases. |
petrochenkov
commented
Aug 1, 2026
@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.
petrochenkov
commented
Aug 1, 2026
Needs a rebase first. |
This comment has been minimized.
This comment has been minimized.
0929cce to
6511e1dComparepetrochenkov
commented
Aug 2, 2026
@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.
…<try> perf: Cache already-checked types in the privacy visitor
This comment has been minimized.
This comment has been minimized.
rust-timer
commented
Aug 2, 2026
Finished benchmarking commit (278a694): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. @bors rollup=never rustc-perf 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 0.4%, secondary -0.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 0.0%, secondary 0.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 488.148s -> 489.283s (0.23%) |
petrochenkov
commented
Aug 3, 2026
I see how the type caching can be useful, but I'm not sure how much the whole |
rustbot
commented
Aug 3, 2026
Reminder, once the PR becomes ready for a review, use |
6511e1d to
da4563aComparexmakro
commented
Aug 3, 2026
You are right, I measured a bit more and the majority of the win is type caching. I updated the PR to type cache only. |
petrochenkov
commented
Aug 3, 2026
@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.
…<try> perf: Cache already-checked types in the privacy visitor
This comment has been minimized.
This comment has been minimized.
rust-timer
commented
Aug 3, 2026
Finished benchmarking commit (5b9f7d9): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. @bors rollup=never rustc-perf 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 -0.9%, secondary -0.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -0.3%, secondary -0.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 491.018s -> 490.335s (-0.14%) |
petrochenkov
commented
Aug 4, 2026
Mostly the same as with Could you make a PR with |
JonathanBrouwer
commented
Aug 4, 2026
@bors rollup=iffy |
…uwer Rollup of 10 perf-sensitive pull requests Successful merges: - #157281 (perf: skip irrelevant foreign impls when building the specialization graph) - #159403 (Next steps for FnDef binder changes (instantiate most FnDef binders)) - #159763 (Optimize crate resolution for large workspace) - #160033 (Speed up `EverInitializedPlaces`) - #160268 (perf: store the fulfillment engine inline in ObligationCtxt) - #160317 (perf: Cache already-checked types in the privacy visitor) - #160399 (interpret: skip deref-projection validity checks when they are not needed) - #160451 (Deduplicate target and host filesearch) - #160453 (Add fast path to `escape_string_symbol`) - #160454 (Add offload guard flags to typeck to prevent perf regressions)
Uh oh!
There was an error while loading. Please reload this page.
Rollup merge of #160317 - xmakro:perf/privacy-accessible-type-cache, r=petrochenkov perf: Cache already-checked types in the privacy visitor The privacy checker walks the full type of every expression and pattern in a module, re-walking the same type once per node it appears on. This caches the types that walked clean (no privacy error) and skips them next time. A walk's result depends only on the interned type and the fixed module being checked, so a type that walks clean once walks clean everywhere. Only clean walks are cached, so nothing is lost: a type that errors is never cached and still fires at every span, and no dep-graph edges are dropped since the full walk already ran once in the same query.
JonathanBrouwer
commented
Aug 5, 2026
Verifying that actual perf results after merge match expected results |
This comment has been minimized.
This comment has been minimized.
…uwer Rollup of 10 perf-sensitive pull requests Successful merges: - rust-lang/rust#157281 (perf: skip irrelevant foreign impls when building the specialization graph) - rust-lang/rust#159403 (Next steps for FnDef binder changes (instantiate most FnDef binders)) - rust-lang/rust#159763 (Optimize crate resolution for large workspace) - rust-lang/rust#160033 (Speed up `EverInitializedPlaces`) - rust-lang/rust#160268 (perf: store the fulfillment engine inline in ObligationCtxt) - rust-lang/rust#160317 (perf: Cache already-checked types in the privacy visitor) - rust-lang/rust#160399 (interpret: skip deref-projection validity checks when they are not needed) - rust-lang/rust#160451 (Deduplicate target and host filesearch) - rust-lang/rust#160453 (Add fast path to `escape_string_symbol`) - rust-lang/rust#160454 (Add offload guard flags to typeck to prevent perf regressions)
rust-timer
commented
Aug 5, 2026
Finished benchmarking commit (76908bc): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. @bors rollup=never rustc-perf 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 -0.7%, secondary -2.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 0.3%, secondary -0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 489.577s -> 496.509s (1.42%) |
xmakro
commented
Aug 5, 2026
I benchmarked the GenericArgs cache on top of the merged type cache with a local rustc-perf run. The result was neutral, nothing moved significantly. Most of what the cache hits is the interned empty GenericArgs list, whose walk is nearly free. So this is not worth a PR. Thanks for spotting this in the original PR. |
…uwer Rollup of 10 perf-sensitive pull requests Successful merges: - rust-lang/rust#157281 (perf: skip irrelevant foreign impls when building the specialization graph) - rust-lang/rust#159403 (Next steps for FnDef binder changes (instantiate most FnDef binders)) - rust-lang/rust#159763 (Optimize crate resolution for large workspace) - rust-lang/rust#160033 (Speed up `EverInitializedPlaces`) - rust-lang/rust#160268 (perf: store the fulfillment engine inline in ObligationCtxt) - rust-lang/rust#160317 (perf: Cache already-checked types in the privacy visitor) - rust-lang/rust#160399 (interpret: skip deref-projection validity checks when they are not needed) - rust-lang/rust#160451 (Deduplicate target and host filesearch) - rust-lang/rust#160453 (Add fast path to `escape_string_symbol`) - rust-lang/rust#160454 (Add offload guard flags to typeck to prevent perf regressions)
View all comments
The privacy checker walks the full type of every expression and pattern in a module, re-walking the same type once per node it appears on. This caches the types that walked clean (no privacy error) and skips them next time. A walk's result depends only on the interned type and the fixed module being checked, so a type that walks clean once walks clean everywhere.
Only clean walks are cached, so nothing is lost: a type that errors is never cached and still fires at every span, and no dep-graph edges are dropped since the full walk already ran once in the same query.