Significantly optimize significant_drop_tightening
#10533
Conversation
|
r? @dswij (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
the
S-waiting-on-review
The lint is very slow as it doesn't cache the deeply nested check for the attribute. If we cache it, we can reduce the time spent on checking `rustc_borrowck` from 28s to 9s, which is a nice improvement. In the profile, the time inside `has_sig_drop_attr` goes from 66% to 0.2%, which is a lot more reasonable. See the PR for nice graphs.
|
I'd also like to nominate this for a beta backport (if the lint is in beta already, which I think it is), as this seems to affect uses where nursery was never enabled (which is kinda weird?). |
| /// Auxiliary structure used to avoid having to verify the same type multiple times. | ||
| seen_types: FxHashSet<Ty<'tcx>>, | ||
| type_cache: FxHashMap<Ty<'tcx>, bool>, |
There was a problem hiding this comment.
Why do we need both? Is type_cache not enough?
There was a problem hiding this comment.
We need one for cycle detection (during the recursive call) and one for caching (after the recursive call)
That's odd. How does the lint affect non-nursery uses? Does the lint fire even when nursery is not enabled? |
|
@bors r+ |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
dswij
added
beta-nominated
The lint pass still runs but the lint won't be emitted if it's not enabled, related: rust-lang/rust#106983 |
flip1995
added
beta-accepted
[beta] Clippy: backport optimization and ICE fixes The first commit optimizes a lint, that runs noticeably slow and is a perf issue. See rust-lang/rust-clippy#10533 The second commit fixes an ICE that occurred when running Clippy on `gitoxide` besides others. Since this is a rather popular crate, we want to fix this rather sooner than later. See rust-lang/rust-clippy#10403 --- Both commits are already on `master` and deployed on `nightly`.
|
This doesn't require a changelog entry, as it was ported to the same version that introduced the lint :). Thank you for the backport @rustbot label -beta-accepted |
rustbot
removed
the
beta-accepted
The lint is very slow as it doesn't cache the deeply nested check for the attribute. If we cache it, we can reduce the time spent on checking
rustc_borrowckfrom 28s to 9s, which is a nice improvement. In the profile, the time insidehas_sig_drop_attrgoes from 66% to 0.2%, which is a lot more reasonable.Flame graphs
Before (all the tall
clippytowers arehas_sig_drop_attr):After:
Fixes #10532
changelog: [
significant_drop_tightening]: significantly optimized