[clang-tidy] Prefer the faster LLVM ADT sets and maps over std:: ones#174357
[clang-tidy] Prefer the faster LLVM ADT sets and maps over std:: ones#174357localspook merged 4 commits intollvm:mainfrom
std:: ones#174357Conversation
|
@llvm/pr-subscribers-clang-tidy Author: Victor Chernyakin (localspook) ChangesThe LLVM docs give a good description of why hyperfine --shell=none './build/release/bin/clang-tidy --checks=* all_headers.cpp -header-filter=.* -system-headers -- -std=c++23'...the results were: Before: After: Benchmark 1: ./build/release/bin/clang-tidy --checks=* all_headers.cpp -header-filter=.* -system-headers -- -std=c++23
Time (mean ± σ): 51.798 s ± 0.126 s [User: 45.194 s, System: 6.575 s]
Range (min … max): 51.620 s … 51.995 s 10 runs...which is a nice little speedup for just switching some containers. I didn't investigate which checks in particular were the source of the speedup though. Full diff: https://github.com/llvm/llvm-project/pull/174357.diff 11 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h
index 297e7751e4f49..01a4ccdf1e717 100644
--- a/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h
+++ b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h
@@ -39,9 +39,9 @@ class IdDependentBackwardBranchCheck : public ClangTidyCheck {
std::string Message;
};
// Stores the locations where ID-dependent variables are created.
- std::map<const VarDecl *, IdDependencyRecord> IdDepVarsMap;
+ llvm::DenseMap<const VarDecl *, IdDependencyRecord> IdDepVarsMap;
// Stores the locations where ID-dependent fields are created.
- std::map<const FieldDecl *, IdDependencyRecord> IdDepFieldsMap;
+ llvm::DenseMap<const FieldDecl *, IdDependencyRecord> IdDepFieldsMap;
/// Returns an IdDependencyRecord if the Expression contains an ID-dependent
/// variable, returns a nullptr otherwise.
const IdDependencyRecord *hasIdDepVar(const Expr *Expression);
diff --git a/clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.cpp
index 892dc02b02298..c34f656ab4252 100644
--- a/clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.cpp
@@ -48,7 +48,7 @@ class MacroExpansionsWithFileAndLine : public PPCallbacks {
}
}
if (HasFile && HasLine)
- SuppressMacroExpansions->insert(Range);
+ SuppressMacroExpansions->insert({Range.getBegin(), Range.getEnd()});
}
private:
@@ -97,8 +97,7 @@ void LambdaFunctionNameCheck::check(const MatchFinder::MatchResult &Result) {
auto ER =
Result.SourceManager->getImmediateExpansionRange(E->getLocation());
- if (SuppressMacroExpansions.find(ER.getAsRange()) !=
- SuppressMacroExpansions.end()) {
+ if (SuppressMacroExpansions.contains({ER.getBegin(), ER.getEnd()})) {
// This is a macro expansion for which we should not warn.
return;
}
diff --git a/clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h b/clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
index d5655037847d3..3460fa8391223 100644
--- a/clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
@@ -21,14 +21,10 @@ namespace clang::tidy::bugprone {
/// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/lambda-function-name.html
class LambdaFunctionNameCheck : public ClangTidyCheck {
public:
- struct SourceRangeLessThan {
- bool operator()(const SourceRange &L, const SourceRange &R) const {
- if (L.getBegin() == R.getBegin())
- return L.getEnd() < R.getEnd();
- return L.getBegin() < R.getBegin();
- }
- };
- using SourceRangeSet = std::set<SourceRange, SourceRangeLessThan>;
+ // FIXME: This pair should be a SourceRange, but SourceRange doesn't have
+ // a DenseMapInfo specialization.
+ using SourceRangeSet =
+ llvm::DenseSet<std::pair<SourceLocation, SourceLocation>>;
LambdaFunctionNameCheck(StringRef Name, ClangTidyContext *Context);
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
diff --git a/clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp b/clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp
index 41521067be86b..4f66a084dd956 100644
--- a/clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp
+++ b/clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp
@@ -70,12 +70,13 @@ static FixItHint getCallFixItHint(const ObjCMessageExpr *Expr,
StringRef Receiver =
getReceiverString(Expr->getReceiverRange(), SM, LangOpts);
// Some classes should use standard factory methods instead of alloc/init.
- std::map<StringRef, StringRef> ClassToFactoryMethodMap = {{"NSDate", "date"},
- {"NSNull", "null"}};
- auto FoundClassFactory = ClassToFactoryMethodMap.find(Receiver);
- if (FoundClassFactory != ClassToFactoryMethodMap.end()) {
- StringRef ClassName = FoundClassFactory->first;
- StringRef FactorySelector = FoundClassFactory->second;
+ static constexpr std::pair<StringRef, StringRef> ClassToFactoryMethodMap[] = {
+ {"NSDate", "date"}, {"NSNull", "null"}};
+ const auto *FoundClassFactory =
+ llvm::find_if(ClassToFactoryMethodMap,
+ [&](const auto &Entry) { return Entry.first == Receiver; });
+ if (FoundClassFactory != std::end(ClassToFactoryMethodMap)) {
+ const auto &[ClassName, FactorySelector] = *FoundClassFactory;
const std::string NewCall =
std::string(llvm::formatv("[{0} {1}]", ClassName, FactorySelector));
return FixItHint::CreateReplacement(Expr->getSourceRange(), NewCall);
diff --git a/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
index 416aca188e01c..592a4313dc197 100644
--- a/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
@@ -42,7 +42,7 @@ class IncludeOrderPPCallbacks : public PPCallbacks {
};
using FileIncludes = std::vector<IncludeDirective>;
- std::map<clang::FileID, FileIncludes> IncludeDirectives;
+ llvm::DenseMap<FileID, FileIncludes> IncludeDirectives;
bool LookForMainModule = true;
ClangTidyCheck &Check;
diff --git a/clang-tools-extra/clang-tidy/misc/NewDeleteOverloadsCheck.h b/clang-tools-extra/clang-tidy/misc/NewDeleteOverloadsCheck.h
index 9c7aff082f8cd..7053e5d9fc1bc 100644
--- a/clang-tools-extra/clang-tidy/misc/NewDeleteOverloadsCheck.h
+++ b/clang-tools-extra/clang-tidy/misc/NewDeleteOverloadsCheck.h
@@ -10,14 +10,11 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NEWDELETEOVERLOADSCHECK_H
#include "../ClangTidyCheck.h"
-#include "llvm/ADT/SmallVector.h"
-#include <map>
namespace clang::tidy::misc {
class NewDeleteOverloadsCheck : public ClangTidyCheck {
- std::map<const clang::CXXRecordDecl *,
- llvm::SmallVector<const clang::FunctionDecl *, 4>>
+ llvm::DenseMap<const CXXRecordDecl *, SmallVector<const FunctionDecl *, 4>>
Overloads;
public:
diff --git a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
index 47363a24abc14..e8dff1d28a3f2 100644
--- a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -11,6 +11,7 @@
#include "clang/AST/ASTLambda.h"
#include "clang/AST/Attr.h"
#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Basic/SourceManager.h"
@@ -84,12 +85,12 @@ class UnusedParametersCheck::IndexerVisitor
public:
IndexerVisitor(ASTContext &Ctx) { TraverseAST(Ctx); }
- const std::unordered_set<const CallExpr *> &
+ const llvm::SmallPtrSetImpl<const CallExpr *> &
getFnCalls(const FunctionDecl *Fn) {
return Index[Fn->getCanonicalDecl()].Calls;
}
- const std::unordered_set<const DeclRefExpr *> &
+ const llvm::SmallPtrSetImpl<const DeclRefExpr *> &
getOtherRefs(const FunctionDecl *Fn) {
return Index[Fn->getCanonicalDecl()].OtherRefs;
}
@@ -119,11 +120,11 @@ class UnusedParametersCheck::IndexerVisitor
private:
struct IndexEntry {
- std::unordered_set<const CallExpr *> Calls;
- std::unordered_set<const DeclRefExpr *> OtherRefs;
+ llvm::SmallPtrSet<const CallExpr *, 2> Calls;
+ llvm::SmallPtrSet<const DeclRefExpr *, 2> OtherRefs;
};
- std::unordered_map<const FunctionDecl *, IndexEntry> Index;
+ llvm::DenseMap<const FunctionDecl *, IndexEntry> Index;
};
UnusedParametersCheck::~UnusedParametersCheck() = default;
diff --git a/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
index bc450ad4a1f2b..137fdb577fca2 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -21,9 +21,9 @@ namespace clang::tidy::modernize {
static constexpr char SpecialFunction[] = "SpecialFunction";
/// Finds all the named non-static fields of \p Record.
-static std::set<const FieldDecl *>
+static llvm::SmallPtrSet<const FieldDecl *, 0>
getAllNamedFields(const CXXRecordDecl *Record) {
- std::set<const FieldDecl *> Result;
+ llvm::SmallPtrSet<const FieldDecl *, 0> Result;
for (const auto *Field : Record->fields()) {
// Static data members are not in this range.
if (Field->isUnnamedBitField())
@@ -35,8 +35,9 @@ getAllNamedFields(const CXXRecordDecl *Record) {
/// Returns the names of the direct bases of \p Record, both virtual and
/// non-virtual.
-static std::set<const Type *> getAllDirectBases(const CXXRecordDecl *Record) {
- std::set<const Type *> Result;
+static llvm::SmallPtrSet<const Type *, 0>
+getAllDirectBases(const CXXRecordDecl *Record) {
+ llvm::SmallPtrSet<const Type *, 0> Result;
for (auto Base : Record->bases()) {
// CXXBaseSpecifier.
const auto *BaseType = Base.getTypeSourceInfo()->getType().getTypePtr();
diff --git a/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.h b/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.h
index 183f1fa8b8a8e..c99e4650a44a8 100644
--- a/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.h
@@ -57,7 +57,7 @@ class BracesAroundStatementsCheck : public ClangTidyCheck {
return TK_IgnoreUnlessSpelledInSource;
}
- std::set<const Stmt *> ForceBracesStmts;
+ llvm::SmallPtrSet<const Stmt *, 0> ForceBracesStmts;
const unsigned ShortStatementLines;
};
diff --git a/clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.h b/clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.h
index 7dcb16e4253b8..7931b6898bbf5 100644
--- a/clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.h
@@ -39,7 +39,7 @@ class NonConstParameterCheck : public ClangTidyCheck {
};
/// Track all nonconst integer/float parameters.
- std::map<const ParmVarDecl *, ParmInfo> Parameters;
+ llvm::DenseMap<const ParmVarDecl *, ParmInfo> Parameters;
/// Add function parameter.
void addParm(const ParmVarDecl *Parm);
diff --git a/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp b/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
index 59cae88708377..88012c3a4b48e 100644
--- a/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
+++ b/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
@@ -261,9 +261,10 @@ class HeaderGuardPPCallbacks : public PPCallbacks {
std::vector<std::pair<Token, const MacroInfo *>> Macros;
llvm::StringMap<const FileEntry *> Files;
- std::map<const IdentifierInfo *, std::pair<SourceLocation, SourceLocation>>
+ llvm::DenseMap<const IdentifierInfo *,
+ std::pair<SourceLocation, SourceLocation>>
Ifndefs;
- std::map<SourceLocation, SourceLocation> EndIfs;
+ llvm::DenseMap<SourceLocation, SourceLocation> EndIfs;
Preprocessor *PP;
HeaderGuardCheck *Check;
|
|
@llvm/pr-subscribers-clang-tools-extra Author: Victor Chernyakin (localspook) ChangesThe LLVM docs give a good description of why hyperfine --shell=none './build/release/bin/clang-tidy --checks=* all_headers.cpp -header-filter=.* -system-headers -- -std=c++23'...the results were: Before: After: Benchmark 1: ./build/release/bin/clang-tidy --checks=* all_headers.cpp -header-filter=.* -system-headers -- -std=c++23
Time (mean ± σ): 51.798 s ± 0.126 s [User: 45.194 s, System: 6.575 s]
Range (min … max): 51.620 s … 51.995 s 10 runs...which is a nice little speedup for just switching some containers. I didn't investigate which checks in particular were the source of the speedup though. Full diff: https://github.com/llvm/llvm-project/pull/174357.diff 11 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h
index 297e7751e4f49..01a4ccdf1e717 100644
--- a/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h
+++ b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h
@@ -39,9 +39,9 @@ class IdDependentBackwardBranchCheck : public ClangTidyCheck {
std::string Message;
};
// Stores the locations where ID-dependent variables are created.
- std::map<const VarDecl *, IdDependencyRecord> IdDepVarsMap;
+ llvm::DenseMap<const VarDecl *, IdDependencyRecord> IdDepVarsMap;
// Stores the locations where ID-dependent fields are created.
- std::map<const FieldDecl *, IdDependencyRecord> IdDepFieldsMap;
+ llvm::DenseMap<const FieldDecl *, IdDependencyRecord> IdDepFieldsMap;
/// Returns an IdDependencyRecord if the Expression contains an ID-dependent
/// variable, returns a nullptr otherwise.
const IdDependencyRecord *hasIdDepVar(const Expr *Expression);
diff --git a/clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.cpp
index 892dc02b02298..c34f656ab4252 100644
--- a/clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.cpp
@@ -48,7 +48,7 @@ class MacroExpansionsWithFileAndLine : public PPCallbacks {
}
}
if (HasFile && HasLine)
- SuppressMacroExpansions->insert(Range);
+ SuppressMacroExpansions->insert({Range.getBegin(), Range.getEnd()});
}
private:
@@ -97,8 +97,7 @@ void LambdaFunctionNameCheck::check(const MatchFinder::MatchResult &Result) {
auto ER =
Result.SourceManager->getImmediateExpansionRange(E->getLocation());
- if (SuppressMacroExpansions.find(ER.getAsRange()) !=
- SuppressMacroExpansions.end()) {
+ if (SuppressMacroExpansions.contains({ER.getBegin(), ER.getEnd()})) {
// This is a macro expansion for which we should not warn.
return;
}
diff --git a/clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h b/clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
index d5655037847d3..3460fa8391223 100644
--- a/clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/LambdaFunctionNameCheck.h
@@ -21,14 +21,10 @@ namespace clang::tidy::bugprone {
/// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/lambda-function-name.html
class LambdaFunctionNameCheck : public ClangTidyCheck {
public:
- struct SourceRangeLessThan {
- bool operator()(const SourceRange &L, const SourceRange &R) const {
- if (L.getBegin() == R.getBegin())
- return L.getEnd() < R.getEnd();
- return L.getBegin() < R.getBegin();
- }
- };
- using SourceRangeSet = std::set<SourceRange, SourceRangeLessThan>;
+ // FIXME: This pair should be a SourceRange, but SourceRange doesn't have
+ // a DenseMapInfo specialization.
+ using SourceRangeSet =
+ llvm::DenseSet<std::pair<SourceLocation, SourceLocation>>;
LambdaFunctionNameCheck(StringRef Name, ClangTidyContext *Context);
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
diff --git a/clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp b/clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp
index 41521067be86b..4f66a084dd956 100644
--- a/clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp
+++ b/clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp
@@ -70,12 +70,13 @@ static FixItHint getCallFixItHint(const ObjCMessageExpr *Expr,
StringRef Receiver =
getReceiverString(Expr->getReceiverRange(), SM, LangOpts);
// Some classes should use standard factory methods instead of alloc/init.
- std::map<StringRef, StringRef> ClassToFactoryMethodMap = {{"NSDate", "date"},
- {"NSNull", "null"}};
- auto FoundClassFactory = ClassToFactoryMethodMap.find(Receiver);
- if (FoundClassFactory != ClassToFactoryMethodMap.end()) {
- StringRef ClassName = FoundClassFactory->first;
- StringRef FactorySelector = FoundClassFactory->second;
+ static constexpr std::pair<StringRef, StringRef> ClassToFactoryMethodMap[] = {
+ {"NSDate", "date"}, {"NSNull", "null"}};
+ const auto *FoundClassFactory =
+ llvm::find_if(ClassToFactoryMethodMap,
+ [&](const auto &Entry) { return Entry.first == Receiver; });
+ if (FoundClassFactory != std::end(ClassToFactoryMethodMap)) {
+ const auto &[ClassName, FactorySelector] = *FoundClassFactory;
const std::string NewCall =
std::string(llvm::formatv("[{0} {1}]", ClassName, FactorySelector));
return FixItHint::CreateReplacement(Expr->getSourceRange(), NewCall);
diff --git a/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
index 416aca188e01c..592a4313dc197 100644
--- a/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
@@ -42,7 +42,7 @@ class IncludeOrderPPCallbacks : public PPCallbacks {
};
using FileIncludes = std::vector<IncludeDirective>;
- std::map<clang::FileID, FileIncludes> IncludeDirectives;
+ llvm::DenseMap<FileID, FileIncludes> IncludeDirectives;
bool LookForMainModule = true;
ClangTidyCheck &Check;
diff --git a/clang-tools-extra/clang-tidy/misc/NewDeleteOverloadsCheck.h b/clang-tools-extra/clang-tidy/misc/NewDeleteOverloadsCheck.h
index 9c7aff082f8cd..7053e5d9fc1bc 100644
--- a/clang-tools-extra/clang-tidy/misc/NewDeleteOverloadsCheck.h
+++ b/clang-tools-extra/clang-tidy/misc/NewDeleteOverloadsCheck.h
@@ -10,14 +10,11 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NEWDELETEOVERLOADSCHECK_H
#include "../ClangTidyCheck.h"
-#include "llvm/ADT/SmallVector.h"
-#include <map>
namespace clang::tidy::misc {
class NewDeleteOverloadsCheck : public ClangTidyCheck {
- std::map<const clang::CXXRecordDecl *,
- llvm::SmallVector<const clang::FunctionDecl *, 4>>
+ llvm::DenseMap<const CXXRecordDecl *, SmallVector<const FunctionDecl *, 4>>
Overloads;
public:
diff --git a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
index 47363a24abc14..e8dff1d28a3f2 100644
--- a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -11,6 +11,7 @@
#include "clang/AST/ASTLambda.h"
#include "clang/AST/Attr.h"
#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Basic/SourceManager.h"
@@ -84,12 +85,12 @@ class UnusedParametersCheck::IndexerVisitor
public:
IndexerVisitor(ASTContext &Ctx) { TraverseAST(Ctx); }
- const std::unordered_set<const CallExpr *> &
+ const llvm::SmallPtrSetImpl<const CallExpr *> &
getFnCalls(const FunctionDecl *Fn) {
return Index[Fn->getCanonicalDecl()].Calls;
}
- const std::unordered_set<const DeclRefExpr *> &
+ const llvm::SmallPtrSetImpl<const DeclRefExpr *> &
getOtherRefs(const FunctionDecl *Fn) {
return Index[Fn->getCanonicalDecl()].OtherRefs;
}
@@ -119,11 +120,11 @@ class UnusedParametersCheck::IndexerVisitor
private:
struct IndexEntry {
- std::unordered_set<const CallExpr *> Calls;
- std::unordered_set<const DeclRefExpr *> OtherRefs;
+ llvm::SmallPtrSet<const CallExpr *, 2> Calls;
+ llvm::SmallPtrSet<const DeclRefExpr *, 2> OtherRefs;
};
- std::unordered_map<const FunctionDecl *, IndexEntry> Index;
+ llvm::DenseMap<const FunctionDecl *, IndexEntry> Index;
};
UnusedParametersCheck::~UnusedParametersCheck() = default;
diff --git a/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
index bc450ad4a1f2b..137fdb577fca2 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -21,9 +21,9 @@ namespace clang::tidy::modernize {
static constexpr char SpecialFunction[] = "SpecialFunction";
/// Finds all the named non-static fields of \p Record.
-static std::set<const FieldDecl *>
+static llvm::SmallPtrSet<const FieldDecl *, 0>
getAllNamedFields(const CXXRecordDecl *Record) {
- std::set<const FieldDecl *> Result;
+ llvm::SmallPtrSet<const FieldDecl *, 0> Result;
for (const auto *Field : Record->fields()) {
// Static data members are not in this range.
if (Field->isUnnamedBitField())
@@ -35,8 +35,9 @@ getAllNamedFields(const CXXRecordDecl *Record) {
/// Returns the names of the direct bases of \p Record, both virtual and
/// non-virtual.
-static std::set<const Type *> getAllDirectBases(const CXXRecordDecl *Record) {
- std::set<const Type *> Result;
+static llvm::SmallPtrSet<const Type *, 0>
+getAllDirectBases(const CXXRecordDecl *Record) {
+ llvm::SmallPtrSet<const Type *, 0> Result;
for (auto Base : Record->bases()) {
// CXXBaseSpecifier.
const auto *BaseType = Base.getTypeSourceInfo()->getType().getTypePtr();
diff --git a/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.h b/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.h
index 183f1fa8b8a8e..c99e4650a44a8 100644
--- a/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.h
@@ -57,7 +57,7 @@ class BracesAroundStatementsCheck : public ClangTidyCheck {
return TK_IgnoreUnlessSpelledInSource;
}
- std::set<const Stmt *> ForceBracesStmts;
+ llvm::SmallPtrSet<const Stmt *, 0> ForceBracesStmts;
const unsigned ShortStatementLines;
};
diff --git a/clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.h b/clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.h
index 7dcb16e4253b8..7931b6898bbf5 100644
--- a/clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/NonConstParameterCheck.h
@@ -39,7 +39,7 @@ class NonConstParameterCheck : public ClangTidyCheck {
};
/// Track all nonconst integer/float parameters.
- std::map<const ParmVarDecl *, ParmInfo> Parameters;
+ llvm::DenseMap<const ParmVarDecl *, ParmInfo> Parameters;
/// Add function parameter.
void addParm(const ParmVarDecl *Parm);
diff --git a/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp b/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
index 59cae88708377..88012c3a4b48e 100644
--- a/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
+++ b/clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
@@ -261,9 +261,10 @@ class HeaderGuardPPCallbacks : public PPCallbacks {
std::vector<std::pair<Token, const MacroInfo *>> Macros;
llvm::StringMap<const FileEntry *> Files;
- std::map<const IdentifierInfo *, std::pair<SourceLocation, SourceLocation>>
+ llvm::DenseMap<const IdentifierInfo *,
+ std::pair<SourceLocation, SourceLocation>>
Ifndefs;
- std::map<SourceLocation, SourceLocation> EndIfs;
+ llvm::DenseMap<SourceLocation, SourceLocation> EndIfs;
Preprocessor *PP;
HeaderGuardCheck *Check;
|
|
Personally I think this is a micro-optimization that makes the code less readable (have to know which ADT type to use, have to switch to .begin()/.end(), etc) and increases the possibilities of bugs (e.g Impl vs non-Impl classes). People will continue to use the STL containers anyway, since that's the most natural choice and there's no enforcement. There's also history of replacing ADT types with STL ones (e.g. std::optional). So I would be softly opposed to this change, but if other people believe it's worth it I'm ok with merging. |
To confirm, you're referring to changes like these? -SuppressMacroExpansions->insert(Range);
+SuppressMacroExpansions->insert({Range.getBegin(), Range.getEnd()});That's a consequence of there being no
True, but that's (AFAIK) in cases where the LLVM alternatives were basically identical to the
(see response under the review comment) |
Would be nice but maybe it can just be done later, no blocker for this patch! Thanks for explaining. |
|
I went and added it in #174524 |
…es (llvm#174357) The LLVM docs give a good description of [why `std::` containers are slower than LLVM alternatives](https://llvm.org/docs/ProgrammersManual.html#set). To see what difference switching to the LLVM ones made, I [reused the approach](llvm#174237 (comment)) of measuring how long it takes to run all checks over all standard library headers (MSVC STL in my case). Using hyperfine (which basically runs a program multiple times and computes how long it took): ```sh hyperfine --shell=none './build/release/bin/clang-tidy --checks=* all_headers.cpp -header-filter=.* -system-headers -- -std=c++23' ``` ...the results were: Before: ``` Benchmark 1: ./build/release/bin/clang-tidy --checks=* all_headers.cpp -header-filter=.* -system-headers -- -std=c++23 Time (mean ± σ): 53.253 s ± 0.089 s [User: 46.480 s, System: 6.748 s] Range (min … max): 53.118 s … 53.440 s 10 runs ``` After: ```txt Benchmark 1: ./build/release/bin/clang-tidy --checks=* all_headers.cpp -header-filter=.* -system-headers -- -std=c++23 Time (mean ± σ): 51.798 s ± 0.126 s [User: 45.194 s, System: 6.575 s] Range (min … max): 51.620 s … 51.995 s 10 runs ``` ...which is a nice little speedup for just switching some containers. I didn't investigate which checks in particular were the source of the speedup though.
…es (llvm#174357) The LLVM docs give a good description of [why `std::` containers are slower than LLVM alternatives](https://llvm.org/docs/ProgrammersManual.html#set). To see what difference switching to the LLVM ones made, I [reused the approach](llvm#174237 (comment)) of measuring how long it takes to run all checks over all standard library headers (MSVC STL in my case). Using hyperfine (which basically runs a program multiple times and computes how long it took): ```sh hyperfine --shell=none './build/release/bin/clang-tidy --checks=* all_headers.cpp -header-filter=.* -system-headers -- -std=c++23' ``` ...the results were: Before: ``` Benchmark 1: ./build/release/bin/clang-tidy --checks=* all_headers.cpp -header-filter=.* -system-headers -- -std=c++23 Time (mean ± σ): 53.253 s ± 0.089 s [User: 46.480 s, System: 6.748 s] Range (min … max): 53.118 s … 53.440 s 10 runs ``` After: ```txt Benchmark 1: ./build/release/bin/clang-tidy --checks=* all_headers.cpp -header-filter=.* -system-headers -- -std=c++23 Time (mean ± σ): 51.798 s ± 0.126 s [User: 45.194 s, System: 6.575 s] Range (min … max): 51.620 s … 51.995 s 10 runs ``` ...which is a nice little speedup for just switching some containers. I didn't investigate which checks in particular were the source of the speedup though.
This is currently one of our most expensive checks according to `--enable-check-profile`. I measured using [the usual setup](#174357 (comment)): ```sh hyperfine \ --shell=none \ --prepare='cmake --build build/release --target clang-tidy' \ './build/release/bin/clang-tidy --checks=-*,readability-container-contains all_headers.cpp -header-filter=.* -system-headers -- -std=c++23 -fno-delayed-template-parsing' ``` First, the status quo: ```txt Time (mean ± σ): 5.243 s ± 0.158 s [User: 4.964 s, System: 0.248 s] Range (min … max): 5.133 s … 5.612 s 10 runs ``` This PR improves that in two independent commits. The first commit changes the default traversal mode from `TK_AsIs` to `TK_IgnoreUnlessSpelledInSource`. Result: ```txt Time (mean ± σ): 4.554 s ± 0.100 s [User: 4.273 s, System: 0.248 s] Range (min … max): 4.442 s … 4.785 s 10 runs ``` The second commit merges all 12 of the check's `BinaryOperation` matchers into one, because I found that each additional registered `BinaryOperation` matcher has pretty extreme overhead. Result: ```txt Time (mean ± σ): 3.480 s ± 0.121 s [User: 3.264 s, System: 0.186 s] Range (min … max): 3.400 s … 3.798 s 10 runs ```
…75121) This is currently one of our most expensive checks according to `--enable-check-profile`. I measured using [the usual setup](llvm/llvm-project#174357 (comment)): ```sh hyperfine \ --shell=none \ --prepare='cmake --build build/release --target clang-tidy' \ './build/release/bin/clang-tidy --checks=-*,readability-container-contains all_headers.cpp -header-filter=.* -system-headers -- -std=c++23 -fno-delayed-template-parsing' ``` First, the status quo: ```txt Time (mean ± σ): 5.243 s ± 0.158 s [User: 4.964 s, System: 0.248 s] Range (min … max): 5.133 s … 5.612 s 10 runs ``` This PR improves that in two independent commits. The first commit changes the default traversal mode from `TK_AsIs` to `TK_IgnoreUnlessSpelledInSource`. Result: ```txt Time (mean ± σ): 4.554 s ± 0.100 s [User: 4.273 s, System: 0.248 s] Range (min … max): 4.442 s … 4.785 s 10 runs ``` The second commit merges all 12 of the check's `BinaryOperation` matchers into one, because I found that each additional registered `BinaryOperation` matcher has pretty extreme overhead. Result: ```txt Time (mean ± σ): 3.480 s ± 0.121 s [User: 3.264 s, System: 0.186 s] Range (min … max): 3.400 s … 3.798 s 10 runs ```
This is currently one of our most expensive checks according to `--enable-check-profile`. I measured using [the usual setup](llvm/llvm-project#174357 (comment)): ```sh hyperfine \ --shell=none \ --prepare='cmake --build build/release --target clang-tidy' \ './build/release/bin/clang-tidy --checks=-*,readability-container-contains all_headers.cpp -header-filter=.* -system-headers -- -std=c++23 -fno-delayed-template-parsing' ``` First, the status quo: ```txt Time (mean ± σ): 5.243 s ± 0.158 s [User: 4.964 s, System: 0.248 s] Range (min … max): 5.133 s … 5.612 s 10 runs ``` This PR improves that in two independent commits. The first commit changes the default traversal mode from `TK_AsIs` to `TK_IgnoreUnlessSpelledInSource`. Result: ```txt Time (mean ± σ): 4.554 s ± 0.100 s [User: 4.273 s, System: 0.248 s] Range (min … max): 4.442 s … 4.785 s 10 runs ``` The second commit merges all 12 of the check's `BinaryOperation` matchers into one, because I found that each additional registered `BinaryOperation` matcher has pretty extreme overhead. Result: ```txt Time (mean ± σ): 3.480 s ± 0.121 s [User: 3.264 s, System: 0.186 s] Range (min … max): 3.400 s … 3.798 s 10 runs ```
This is currently one of our most expensive checks according to `--enable-check-profile`. I measured using [the usual setup](llvm#174357 (comment)): ```sh hyperfine \ --shell=none \ --prepare='cmake --build build/release --target clang-tidy' \ './build/release/bin/clang-tidy --checks=-*,readability-container-contains all_headers.cpp -header-filter=.* -system-headers -- -std=c++23 -fno-delayed-template-parsing' ``` First, the status quo: ```txt Time (mean ± σ): 5.243 s ± 0.158 s [User: 4.964 s, System: 0.248 s] Range (min … max): 5.133 s … 5.612 s 10 runs ``` This PR improves that in two independent commits. The first commit changes the default traversal mode from `TK_AsIs` to `TK_IgnoreUnlessSpelledInSource`. Result: ```txt Time (mean ± σ): 4.554 s ± 0.100 s [User: 4.273 s, System: 0.248 s] Range (min … max): 4.442 s … 4.785 s 10 runs ``` The second commit merges all 12 of the check's `BinaryOperation` matchers into one, because I found that each additional registered `BinaryOperation` matcher has pretty extreme overhead. Result: ```txt Time (mean ± σ): 3.480 s ± 0.121 s [User: 3.264 s, System: 0.186 s] Range (min … max): 3.400 s … 3.798 s 10 runs ```
This is currently one of our most expensive checks according to `--enable-check-profile`. I measured using [the usual setup](llvm#174357 (comment)): ```sh hyperfine \ --shell=none \ --prepare='cmake --build build/release --target clang-tidy' \ './build/release/bin/clang-tidy --checks=-*,readability-container-contains all_headers.cpp -header-filter=.* -system-headers -- -std=c++23 -fno-delayed-template-parsing' ``` First, the status quo: ```txt Time (mean ± σ): 5.243 s ± 0.158 s [User: 4.964 s, System: 0.248 s] Range (min … max): 5.133 s … 5.612 s 10 runs ``` This PR improves that in two independent commits. The first commit changes the default traversal mode from `TK_AsIs` to `TK_IgnoreUnlessSpelledInSource`. Result: ```txt Time (mean ± σ): 4.554 s ± 0.100 s [User: 4.273 s, System: 0.248 s] Range (min … max): 4.442 s … 4.785 s 10 runs ``` The second commit merges all 12 of the check's `BinaryOperation` matchers into one, because I found that each additional registered `BinaryOperation` matcher has pretty extreme overhead. Result: ```txt Time (mean ± σ): 3.480 s ± 0.121 s [User: 3.264 s, System: 0.186 s] Range (min … max): 3.400 s … 3.798 s 10 runs ```
This is currently one of our most expensive checks according to `--enable-check-profile`. I measured using [the usual setup](llvm#174357 (comment)): ```sh hyperfine \ --shell=none \ --prepare='cmake --build build/release --target clang-tidy' \ './build/release/bin/clang-tidy --checks=-*,readability-container-contains all_headers.cpp -header-filter=.* -system-headers -- -std=c++23 -fno-delayed-template-parsing' ``` First, the status quo: ```txt Time (mean ± σ): 5.243 s ± 0.158 s [User: 4.964 s, System: 0.248 s] Range (min … max): 5.133 s … 5.612 s 10 runs ``` This PR improves that in two independent commits. The first commit changes the default traversal mode from `TK_AsIs` to `TK_IgnoreUnlessSpelledInSource`. Result: ```txt Time (mean ± σ): 4.554 s ± 0.100 s [User: 4.273 s, System: 0.248 s] Range (min … max): 4.442 s … 4.785 s 10 runs ``` The second commit merges all 12 of the check's `BinaryOperation` matchers into one, because I found that each additional registered `BinaryOperation` matcher has pretty extreme overhead. Result: ```txt Time (mean ± σ): 3.480 s ± 0.121 s [User: 3.264 s, System: 0.186 s] Range (min … max): 3.400 s … 3.798 s 10 runs ```
The LLVM docs give a good description of why
std::containers are slower than LLVM alternatives. To see what difference switching to the LLVM ones made, I reused the approach of measuring how long it takes to run all checks over all standard library headers (MSVC STL in my case). Using hyperfine (which basically runs a program multiple times and computes how long it took):hyperfine --shell=none './build/release/bin/clang-tidy --checks=* all_headers.cpp -header-filter=.* -system-headers -- -std=c++23'...the results were:
Before:
After:
...which is a nice little speedup for just switching some containers. I didn't investigate which checks in particular were the source of the speedup though.