Skip to content

Huge CPU and RAM usage in redundant_clone/possible_borrower on long functions #10134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mwkmwkmwk opened this issue Dec 30, 2022 · 15 comments
Closed
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@mwkmwkmwk
Copy link

mwkmwkmwk commented Dec 30, 2022

Summary

Clippy takes a very large amount of time and RAM to analyze the following code. The time is spent in a single invocation of <rustc_mir_dataflow::framework::engine::Engine<clippy_utils::mir::possible_borrower::PossibleBorrowerAnalysis>>::iterate_to_fixpoint:

[...]
#6  0x00005567f6f1a249 in <rustc_mir_dataflow::framework::engine::Engine<clippy_utils::mir::possible_borrower::PossibleBorrowerAnalysis>>::iterate_to_fixpoint ()
#7  0x00005567f6f79839 in <clippy_utils::mir::possible_borrower::PossibleBorrowerMap>::new ()
#8  0x00005567f6e4d222 in <clippy_lints::redundant_clone::RedundantClone as rustc_lint::passes::LateLintPass>::check_fn ()
[...]

The bug happens with nightly-2022-12-30, while nightly-2022-12-29 is fine (clippy finishes near-instantly).

Reproducer

I tried this code:

fn meow(_s: impl AsRef<str>) {
}

macro_rules! quad {
    ($x:stmt) => {
        $x
        $x
        $x
        $x
    };
}

fn main() {
    let i = 0;
    quad!(quad!(quad!(quad!(quad!(meow(format!("abc{i}")))))));
}

(adjust amount of quad! to taste; the above will reliably OOM my machine)

I expected to see this happen:

Clippy finishes near-instantly.

Instead, this happened:

Clippy crashes with OOM after a while.

Version

rustc 1.68.0-nightly (ad8ae0504 2022-12-29)
binary: rustc
commit-hash: ad8ae0504c54bc2bd8306abfcfe8546c1bb16a49
commit-date: 2022-12-29
host: x86_64-unknown-linux-gnu
release: 1.68.0-nightly
LLVM version: 15.0.6

Additional Labels

No response

@mwkmwkmwk mwkmwkmwk added the C-bug Category: Clippy is not doing the correct thing label Dec 30, 2022
@llogiq
Copy link
Contributor

llogiq commented Dec 30, 2022

So you have 1024 meow-calls? Sounds like my cat standing in front of the patio door.

@kayabaNerve
Copy link

I believe I have this same bug. January 1st nightly clippy is killed after ballooning to 6.1 GB in a few seconds. stable only uses 700 MB. Dec 30th also fails. Dec 29th is fine. While I do have macros, I have no so recursive (AFAIK). Unfortunately, I do not know what in my (rather large) library could serve as MWE.

https://github.com/serai-dex/serai/tree/develop/coins/monero in case it is somehow helpful. cargo +nightly-2022-12-29 clippy (ignoring features and tests) is enough to trigger the monstrous memory usage.

@Jarcho
Copy link
Contributor

Jarcho commented Jan 2, 2023

Probably due to #9701. Ping @smoelius.

I might have time to look at it in the next couple of days.

@smoelius
Copy link
Contributor

smoelius commented Jan 2, 2023

Probably due to #9701.

No doubt.

One thing I noticed is that the meow example involves many singleton sets. So, storing the borrowers in HybridBitSets rather than BitSets helps (see #10144). On my laptop, Clippy gets through the example in a few seconds. But if add another quad, I wind up killing it after 20 seconds or so.

I experimented with a few variants of this idea. For example, using copy-on-write HybridBitSets seems to help a little, but not dramatically.

I am open to ideas.

@llogiq
Copy link
Contributor

llogiq commented Jan 2, 2023

Perhaps we need a different approach that calculates the borrowers for a function once and caches that while linting the function to better amortize the work? To me it looks like we may be accidentally quadratic even with a less memory hungry representation.

Btw. I'm curious: How do hybrid bit sets compare to RoaringBitSets?

@amaanq
Copy link

amaanq commented Jan 4, 2023

Bisected and leaving this log here, still investigating on my end:

git bisect start
# status: waiting for both good and bad commits
# bad: [4fe3727c391ec5bf18019f304d87132a3e3c5211] Auto merge of #9701 - smoelius:improve-possible-borrower, r=Jarcho
git bisect bad 4fe3727c391ec5bf18019f304d87132a3e3c5211
# status: waiting for good commit(s), bad commit known
# bad: [4fe3727c391ec5bf18019f304d87132a3e3c5211] Auto merge of #9701 - smoelius:improve-possible-borrower, r=Jarcho
git bisect bad 4fe3727c391ec5bf18019f304d87132a3e3c5211
# status: waiting for good commit(s), bad commit known
# good: [8a6e6fd623a896a37c1d6defed56b7d98273d4fa] Auto merge of #10056 - koka831:fix/9993, r=Jarcho
git bisect good 8a6e6fd623a896a37c1d6defed56b7d98273d4fa
# bad: [ed519ad746e31f64c4e9255be561785612532d37] Improve `possible_borrower`
git bisect bad ed519ad746e31f64c4e9255be561785612532d37
# bad: [ed519ad746e31f64c4e9255be561785612532d37] Improve `possible_borrower`
git bisect bad ed519ad746e31f64c4e9255be561785612532d37
# good: [1e68973582403f62bf2d55aa2b805b43b4e931cc] Auto merge of #10067 - chansuke:issue-7943, r=giraffate
git bisect good 1e68973582403f62bf2d55aa2b805b43b4e931cc
# good: [c6477eb71188311f01f409da628fab7062697bd7] Add tests
git bisect good c6477eb71188311f01f409da628fab7062697bd7
# first bad commit: [ed519ad746e31f64c4e9255be561785612532d37] Improve `possible_borrower`

ed519ad746e31f64c4e9255be561785612532d37 is the first bad commit
commit ed519ad746e31f64c4e9255be561785612532d37
Author: Samuel Moelius <[email protected]>
Date:   Wed Oct 12 04:34:25 2022 -0400

    Improve `possible_borrower`

 clippy_lints/src/dereference.rs           |   8 +-
 clippy_lints/src/redundant_clone.rs       |   4 +-
 clippy_utils/src/mir/possible_borrower.rs | 265 +++++++++++++++++++-----------
 tests/ui/needless_borrow.fixed            |   2 +-
 tests/ui/needless_borrow.stderr           |   8 +-
 tests/ui/redundant_clone.fixed            |   2 +-
 tests/ui/redundant_clone.stderr           |  14 +-
 7 files changed, 195 insertions(+), 108 deletions(-)

@mwkmwkmwk
Copy link
Author

Second testcase that still OOMs even with #10173:

fn meow(_s: impl AsRef<str>) {}

macro_rules! quad {
    ($x:stmt) => {
        $x
        $x
        $x
        $x
    };
}

fn main() {
    quad!(quad!(quad!(for i in 0..4 {
        quad!(quad!(meow(format!("abc{i}"))));
    })));
}

@smoelius
Copy link
Contributor

smoelius commented Jan 7, 2023

Oof. Thanks, @mwkmwkmwk.

@digama0
Copy link
Contributor

digama0 commented Jan 10, 2023

This is also causing my project's CI to fail. I can minimize if necessary but since there are already a few examples I will wait to see if the fixes for the issues above also solve my problem.

@kayabaNerve
Copy link

Should the problem commit in question be reverted until a fixed version is available, since this has broken multiple projects? I understand nightly doesn't have the safety guarantees of stable, it's nightly, yet some targets require nightly and nightly is currently known to be not functioning to standards.

I'm unsure the exact policies here, just wanted to raise it and get the opinion of someone more educated. I've personally just stuck to 12-29 to avoid it, for now.

@Jarcho
Copy link
Contributor

Jarcho commented Jan 11, 2023

Clippy is normally synced every two weeks with with the main rust repo every two weeks. I believe the next sync is coming up this Thursday, so if it doesn't have a fix before then we can revert it right before the sync.

@BlackDex
Copy link

Same thing happens during a clippy run on Vaultwarden.
Using beta or stable works fine though.

bors added a commit that referenced this issue Jan 12, 2023
Partially revert #9701

This partially reverts #9701 due to #10134

r? `@flip1995`

changelog: None
@Jarcho
Copy link
Contributor

Jarcho commented Jan 13, 2023

Next nightly should be reverted.

@samueltardieu
Copy link
Contributor

Can we close this one?

@BlackDex
Copy link

Can we close this one?

I think so, i have not seen this happen for a while now, it's about 2 years old haha.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing
Projects
None yet
Development

No branches or pull requests

9 participants