[clang-tools-extra] [NFC][clang-tidy]improve performance for misc-unused-using-decls check (PR #78231)
HerrCai0907 wrote: > LGTM. There is open issue for performance of this check. Consider mentioning > this in release notes as some generic performance improvement. I have also seen this issue. But after optimization, this check is still slow. I guess registering too many pattern causes performance issue but I cannot find a good solution to optimize it. https://github.com/llvm/llvm-project/pull/78231 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [NFC][clang-tidy]improve performance for misc-unused-using-decls check (PR #78231)
https://github.com/HerrCai0907 closed https://github.com/llvm/llvm-project/pull/78231 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [NFC][clang-tidy]improve performance for misc-unused-using-decls check (PR #78231)
HerrCai0907 wrote: > Thanks, the code looks good. Do you have any performance data after this > change? I don't run clang-tidy bench for some large project. But I do some test for some auto-generated cpp file which has lots of using, and the execution time reduce from 0.2s to 0.18s in my MAC M1 Pro. https://github.com/llvm/llvm-project/pull/78231 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [NFC][clang-tidy]improve performance for misc-unused-using-decls check (PR #78231)
https://github.com/hokein approved this pull request. Thanks, the code looks good. Do you have any performance data after this change? https://github.com/llvm/llvm-project/pull/78231 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [NFC][clang-tidy]improve performance for misc-unused-using-decls check (PR #78231)
https://github.com/PiotrZSL approved this pull request. LGTM. There is open issue for performance of this check. Consider mentioning this in release notes as some generic performance improvment. Do you have any benchmarks ? https://github.com/llvm/llvm-project/pull/78231 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [NFC][clang-tidy]improve performance for misc-unused-using-decls check (PR #78231)
llvmbot wrote:
@llvm/pr-subscribers-clang-tidy
Author: Congcong Cai (HerrCai0907)
Changes
`UnusedUsingDeclsCheck::removeFromFoundDecls` will be called with high
frequency. At current time it will check every `Context`.
This patch adds a cache to reduce algorithm complexity.
---
Full diff: https://github.com/llvm/llvm-project/pull/78231.diff
2 Files Affected:
- (modified) clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
(+10-4)
- (modified) clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.h (+1)
``diff
diff --git a/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
b/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
index 051375263e53c3..59e487bab31195 100644
--- a/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
@@ -117,8 +117,10 @@ void UnusedUsingDeclsCheck::check(const
MatchFinder::MatchResult ) {
/*SkipTrailingWhitespaceAndNewLine=*/true));
for (const auto *UsingShadow : Using->shadows()) {
const auto *TargetDecl =
UsingShadow->getTargetDecl()->getCanonicalDecl();
- if (shouldCheckDecl(TargetDecl))
+ if (shouldCheckDecl(TargetDecl)) {
Context.UsingTargetDecls.insert(TargetDecl);
+UsingTargetDeclsCache.insert(TargetDecl);
+ }
}
if (!Context.UsingTargetDecls.empty())
Contexts.push_back(Context);
@@ -201,13 +203,16 @@ void UnusedUsingDeclsCheck::check(const
MatchFinder::MatchResult ) {
void UnusedUsingDeclsCheck::removeFromFoundDecls(const Decl *D) {
if (!D)
return;
+ const Decl *CanonicalDecl = D->getCanonicalDecl();
+ if (!UsingTargetDeclsCache.contains(CanonicalDecl))
+return;
// FIXME: Currently, we don't handle the using-decls being used in different
// scopes (such as different namespaces, different functions). Instead of
// giving an incorrect message, we mark all of them as used.
- //
- // FIXME: Use a more efficient way to find a matching context.
for (auto : Contexts) {
-if (Context.UsingTargetDecls.contains(D->getCanonicalDecl()))
+if (Context.IsUsed)
+ continue;
+if (Context.UsingTargetDecls.contains(CanonicalDecl))
Context.IsUsed = true;
}
}
@@ -224,6 +229,7 @@ void UnusedUsingDeclsCheck::onEndOfTranslationUnit() {
}
}
Contexts.clear();
+ UsingTargetDeclsCache.clear();
}
} // namespace clang::tidy::misc
diff --git a/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.h
b/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.h
index 498b3ffd2678c3..7bdaf12e8aecee 100644
--- a/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.h
+++ b/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.h
@@ -49,6 +49,7 @@ class UnusedUsingDeclsCheck : public ClangTidyCheck {
};
std::vector Contexts;
+ llvm::SmallPtrSet UsingTargetDeclsCache;
StringRef RawStringHeaderFileExtensions;
FileExtensionsSet HeaderFileExtensions;
``
https://github.com/llvm/llvm-project/pull/78231
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [NFC][clang-tidy]improve performance for misc-unused-using-decls check (PR #78231)
https://github.com/HerrCai0907 created
https://github.com/llvm/llvm-project/pull/78231
`UnusedUsingDeclsCheck::removeFromFoundDecls` will be called with high
frequency. At current time it will check every `Context`.
This patch adds a cache to reduce algorithm complexity.
>From ebd6f7648c7b4feeb88061b1127d96cf5e23d098 Mon Sep 17 00:00:00 2001
From: Congcong Cai
Date: Tue, 16 Jan 2024 10:51:14 +0800
Subject: [PATCH] [NFC][clang-tidy]improve performance for
misc-unused-using-decls check
`UnusedUsingDeclsCheck::removeFromFoundDecls` will be called with high
frequency.
At current time it will check every `Context`.
This patch adds a cache to reduce algorithm complexitiy.
---
.../clang-tidy/misc/UnusedUsingDeclsCheck.cpp | 14 ++
.../clang-tidy/misc/UnusedUsingDeclsCheck.h| 1 +
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
b/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
index 051375263e53c3..59e487bab31195 100644
--- a/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
@@ -117,8 +117,10 @@ void UnusedUsingDeclsCheck::check(const
MatchFinder::MatchResult ) {
/*SkipTrailingWhitespaceAndNewLine=*/true));
for (const auto *UsingShadow : Using->shadows()) {
const auto *TargetDecl =
UsingShadow->getTargetDecl()->getCanonicalDecl();
- if (shouldCheckDecl(TargetDecl))
+ if (shouldCheckDecl(TargetDecl)) {
Context.UsingTargetDecls.insert(TargetDecl);
+UsingTargetDeclsCache.insert(TargetDecl);
+ }
}
if (!Context.UsingTargetDecls.empty())
Contexts.push_back(Context);
@@ -201,13 +203,16 @@ void UnusedUsingDeclsCheck::check(const
MatchFinder::MatchResult ) {
void UnusedUsingDeclsCheck::removeFromFoundDecls(const Decl *D) {
if (!D)
return;
+ const Decl *CanonicalDecl = D->getCanonicalDecl();
+ if (!UsingTargetDeclsCache.contains(CanonicalDecl))
+return;
// FIXME: Currently, we don't handle the using-decls being used in different
// scopes (such as different namespaces, different functions). Instead of
// giving an incorrect message, we mark all of them as used.
- //
- // FIXME: Use a more efficient way to find a matching context.
for (auto : Contexts) {
-if (Context.UsingTargetDecls.contains(D->getCanonicalDecl()))
+if (Context.IsUsed)
+ continue;
+if (Context.UsingTargetDecls.contains(CanonicalDecl))
Context.IsUsed = true;
}
}
@@ -224,6 +229,7 @@ void UnusedUsingDeclsCheck::onEndOfTranslationUnit() {
}
}
Contexts.clear();
+ UsingTargetDeclsCache.clear();
}
} // namespace clang::tidy::misc
diff --git a/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.h
b/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.h
index 498b3ffd2678c3..7bdaf12e8aecee 100644
--- a/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.h
+++ b/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.h
@@ -49,6 +49,7 @@ class UnusedUsingDeclsCheck : public ClangTidyCheck {
};
std::vector Contexts;
+ llvm::SmallPtrSet UsingTargetDeclsCache;
StringRef RawStringHeaderFileExtensions;
FileExtensionsSet HeaderFileExtensions;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
