Add fast-path when computing the default visibility #131686
Conversation
rustbot
added
S-waiting-on-review
|
As cachegrind showed the small regressions in #130005 to be purely in the query system, I'm fully confident that the new unconditional query call to the expensive query was the source. So, r=me with green perf and/or a cachegrind comparison between this and #130005 (or its parent commit). I expect the "PROGRAM TOTALS" line on a cg report for this PR to be the same as before the change was made. |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-perf
Add fast-path when computing the default visibility This PR adds (or more correctly re-adds the) fast-path when computing the default visibility, by taking advantage of the fact that the "interposable" requested visibility always return the "default" codegen visibility. Should address the small regression observed in rust-lang#131111 (comment). r? `@lqd`
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (701880a): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. @bors rollup=never Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary 1.4%, secondary 2.0%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (secondary -2.4%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 782.104s -> 782.36s (0.03%) |
rustbot
removed
the
S-waiting-on-perf
|
|
bors
added
S-waiting-on-bors
|
Actually, we have the results, and the regressions are mild enough that it's fine to roll this PR up. @bors rollup=maybe |
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#130822 (Add `from_ref` and `from_mut` constructors to `core::ptr::NonNull`.) - rust-lang#131381 (Implement edition 2024 match ergonomics restrictions) - rust-lang#131594 (rustdoc: Rename "object safe" to "dyn compatible") - rust-lang#131686 (Add fast-path when computing the default visibility) - rust-lang#131699 (Try to improve error messages involving aliases in the solver) - rust-lang#131757 (Ignore lint-non-snake-case-crate#proc_macro_ on targets without unwind) - rust-lang#131783 (Fix explicit_iter_loop in rustc_serialize) - rust-lang#131788 (Fix mismatched quotation mark) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#131686 - Urgau:fast-path-vis, r=lqd Add fast-path when computing the default visibility This PR adds (or more correctly re-adds the) fast-path when computing the default visibility, by taking advantage of the fact that the "interposable" requested visibility always return the "default" codegen visibility. Should address the small regression observed in rust-lang#131111 (comment). r? `@lqd`
This PR adds (or more correctly re-adds the) fast-path when computing the default visibility, by taking advantage of the fact that the "interposable" requested visibility always return the "default" codegen visibility.
Should address the small regression observed in #131111 (comment).
r? @lqd