clang-tidy check `misc-unused-using-decls` appears to be quite slow

[firewave]

The misc-unused-using-decls check constantly comes up as one of the top 3 expensive checks in our code base (it usually takes about 5% of the total time). Considering it is not providing that much in terms of its findings compared to other checks it seems a bit excessive.

  ---User Time---   --System Time--   --User+System--   ---Wall Time---  --- Name ---
   0.8125 ( 11.9%)   0.0000 (  0.0%)   0.8125 (  7.4%)   0.7912 (  7.2%)  bugprone-reserved-identifier
   0.4531 (  6.6%)   0.3125 (  7.6%)   0.7656 (  7.0%)   0.6002 (  5.5%)  misc-unused-using-decls
   0.2344 (  3.4%)   0.0938 (  2.3%)   0.3281 (  3.0%)   0.4317 (  3.9%)  bugprone-use-after-move

   0.7969 ( 11.6%)   0.0312 (  0.8%)   0.8281 (  7.5%)   0.8204 (  7.5%)  bugprone-reserved-identifier
   0.2969 (  4.3%)   0.3125 (  7.7%)   0.6094 (  5.6%)   0.6044 (  5.5%)  misc-unused-using-decls
   0.4219 (  6.1%)   0.1094 (  2.7%)   0.5312 (  4.8%)   0.4172 (  3.8%)  bugprone-use-after-move

   0.5781 (  5.0%)   0.4688 (  4.9%)   1.0469 (  5.0%)   1.1171 (  5.3%)  misc-unused-using-decls
   0.8750 (  7.6%)   0.0938 (  1.0%)   0.9688 (  4.6%)   0.9738 (  4.6%)  bugprone-reserved-identifier
   0.7969 (  6.9%)   0.2344 (  2.5%)   1.0312 (  4.9%)   0.8989 (  4.3%)  bugprone-use-after-move

[sean-mcmanus]

@firewave Our team saw the same issue with our project (so we disabled that check in certain scenarios).

[PiotrZSL]

That's on LLVM 17/18 ? Based on how this check is implemented, yes it can be slow & buggy. I will see what I can do.

[sean-mcmanus]

@PiotrZSL I used clang-tidy 17.0.2.

[firewave]

My numbers are from LLVM 18.

[firewave]

Looks like this was fixed by #78231.

CC @HerrCai0907

[HerrCai0907]

Looks like this was fixed by #78231.
yes, but partially.

@firewave Could you re-benchmark with current version?

[firewave]

I am still seeing this:

   ---User Time---   --System Time--   --User+System--   ---Wall Time---  --- Name ---
   0.9531 ( 10.8%)   0.0625 (  1.1%)   1.0156 (  6.9%)   1.0258 (  7.0%)  bugprone-reserved-identifier
   0.4688 (  5.3%)   0.4531 (  7.8%)   0.9219 (  6.3%)   0.8052 (  5.5%)  misc-unused-using-decls
   0.3750 (  4.2%)   0.1406 (  2.4%)   0.5156 (  3.5%)   0.5591 (  3.8%)  bugprone-use-after-move

I am using the binaries from apt.llvm.sh and the commit hash in the version indicates that the changes are included.

Ubuntu clang version 18.0.0 (++20240123053045+8ed1291d96ea-1~exp1~20240123173204.910)

It looks like it is the wrong version but apt.llvm.sh has not yet been adjusted for the version bump. That usually takes a few days.

[PiotrZSL]

800ms for a clang-tidy check is not much, depend on file size.

[firewave]

800ms for a clang-tidy check is not much, depend on file size.

You sure? If you would have 100 of such checks a single file would take 80000ms. But we digress.

The percentage still matches what I reported in the initial report. And it is still one of the slowest checks.

   ---User Time---   --System Time--   --User+System--   ---Wall Time---  --- Name ---
   0.6094 (  4.0%)   0.5469 (  5.7%)   1.1562 (  4.7%)   1.2513 (  5.0%)  misc-unused-using-decls
   0.8125 (  5.4%)   0.2656 (  2.8%)   1.0781 (  4.4%)   1.0980 (  4.4%)  bugprone-use-after-move
   0.9844 (  6.5%)   0.0781 (  0.8%)   1.0625 (  4.3%)   1.0535 (  4.2%)  bugprone-reserved-identifier

   1.6406 (  8.0%)   0.3750 (  2.6%)   2.0156 (  5.8%)   1.8890 (  5.4%)  bugprone-use-after-move
   1.0156 (  5.0%)   0.6562 (  4.6%)   1.6719 (  4.8%)   1.7865 (  5.1%)  misc-unused-using-decls
   0.8438 (  4.1%)   0.3438 (  2.4%)   1.1875 (  3.4%)   1.4063 (  4.0%)  bugprone-standalone-empty

[PiotrZSL]

Lot of code, lot of using directives. This check need to be re-written to use RecursiveASTMatcher like bugprone-reserved-identifier were. misc-unused-using-decls would also benefit from ignoring system headers, and there were WIP change for that, but it stuck as amount of reviewers is ... small.
As for bugprone-standalone-empty, that just need some tuning like TK_IgnoreUnlessSpelledInSource.

From "priority" point of view there are other changes like https://reviews.llvm.org/D150126 that would bring bigger improvements over all checks, not only this one. And other issues. When your points are valid, I just don't think that anyone will spend time on this in near time, unless issue can be reproduced on some bigger files where this check would take more than ~20s, otherwise that's just a noise caused by simply cost of an AST processing.

[firewave]

Possibly improved by #123454.