lebedev.ri created this revision.
lebedev.ri added reviewers: sbenza, alexfh, klimek.
https://reviews.llvm.org/D5911 added support for profiling clang-tidy checks.
It works nice, the tabulated report generated by `clang-tidy
-enable-check-profile` is readable.
Unfortunately, it gets complicated with more than just one source file.
You could run `clang-tidy` on each file, then parse the profiles,
combine (sum) based on the `Name` column, and re-display.
Given that the profiles are display friendly, this is messy. (or is there a
tool i've missed?)
Or, you could run `clang-tidy` only once, on all the sources at at once.
This obviously does not scale well. But one would think at least
it would sidestep the problem of combining the separate reports.
**One would think**. No, it does not. The profiling info is displayed,
only once at the end of `clang-tidy` run, but the info is garbage.
It only contains some portion of the data. I suspect it only contains the last
TU.
This is because `MatchASTVisitor` is kinda smart, it contains it's own local
`llvm::StringMap<llvm::TimeRecord> TimeByBucket;`, which is used,
and at the end, `MatchASTVisitor::~MatchASTVisitor()` "propagates"
the collected data to the specified profiling info storage.
But it overrides it, not appends/combines...
Repository:
rC Clang
https://reviews.llvm.org/D45931
Files:
lib/ASTMatchers/ASTMatchFinder.cpp
unittests/ASTMatchers/ASTMatchersInternalTest.cpp
Index: unittests/ASTMatchers/ASTMatchersInternalTest.cpp
===================================================================
--- unittests/ASTMatchers/ASTMatchersInternalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersInternalTest.cpp
@@ -119,6 +119,46 @@
EXPECT_EQ("MyID", Records.begin()->getKey());
}
+TEST(MatchFinder, CheckProfilingTwoTU) {
+ llvm::StringMap<llvm::TimeRecord> Records;
+
+ MatchFinder::MatchFinderOptions Options0;
+ Options0.CheckProfiling.emplace(Records);
+ MatchFinder Finder0(std::move(Options0));
+
+ struct NamedCallback0 : public MatchFinder::MatchCallback {
+ void run(const MatchFinder::MatchResult &Result) override {}
+ StringRef getID() const override { return "MyID0"; }
+ } Callback0;
+ Finder0.addMatcher(decl(), &Callback0);
+ std::unique_ptr<FrontendActionFactory> Factory0(
+ newFrontendActionFactory(&Finder0));
+
+ MatchFinder::MatchFinderOptions Options1;
+ Options1.CheckProfiling.emplace(Records); // **before** using the first one.
+ MatchFinder Finder1(std::move(Options1));
+
+ struct NamedCallback1 : public MatchFinder::MatchCallback {
+ void run(const MatchFinder::MatchResult &Result) override {}
+ StringRef getID() const override { return "MyID1"; }
+ } Callback1;
+ Finder1.addMatcher(decl(), &Callback1);
+ std::unique_ptr<FrontendActionFactory> Factory1(
+ newFrontendActionFactory(&Finder1));
+
+ ASSERT_TRUE(tooling::runToolOnCode(Factory0->create(), "int x;"));
+
+ EXPECT_EQ(1u, Records.size());
+ EXPECT_EQ("MyID0", Records.begin()->getKey());
+ EXPECT_NE(Records.find("MyID0"), Records.end());
+
+ ASSERT_TRUE(tooling::runToolOnCode(Factory1->create(), "int x;"));
+
+ EXPECT_EQ(2u, Records.size());
+ EXPECT_NE(Records.find("MyID0"), Records.end());
+ EXPECT_NE(Records.find("MyID1"), Records.end());
+}
+
class VerifyStartOfTranslationUnit : public MatchFinder::MatchCallback {
public:
VerifyStartOfTranslationUnit() : Called(false) {}
Index: lib/ASTMatchers/ASTMatchFinder.cpp
===================================================================
--- lib/ASTMatchers/ASTMatchFinder.cpp
+++ lib/ASTMatchers/ASTMatchFinder.cpp
@@ -311,31 +311,26 @@
public ASTMatchFinder {
public:
MatchASTVisitor(const MatchFinder::MatchersByType *Matchers,
- const MatchFinder::MatchFinderOptions &Options)
- : Matchers(Matchers), Options(Options), ActiveASTContext(nullptr) {}
-
- ~MatchASTVisitor() override {
- if (Options.CheckProfiling) {
- Options.CheckProfiling->Records = std::move(TimeByBucket);
- }
+ MatchFinder::MatchFinderOptions &Options)
+ : Matchers(Matchers), ActiveASTContext(nullptr) {
+ if (Options.CheckProfiling.hasValue())
+ TimeByBucket = &(Options.CheckProfiling->Records);
}
void onStartOfTranslationUnit() {
- const bool EnableCheckProfiling = Options.CheckProfiling.hasValue();
TimeBucketRegion Timer;
for (MatchCallback *MC : Matchers->AllCallbacks) {
- if (EnableCheckProfiling)
- Timer.setBucket(&TimeByBucket[MC->getID()]);
+ if (TimeByBucket)
+ Timer.setBucket(&(*TimeByBucket)[MC->getID()]);
MC->onStartOfTranslationUnit();
}
}
void onEndOfTranslationUnit() {
- const bool EnableCheckProfiling = Options.CheckProfiling.hasValue();
TimeBucketRegion Timer;
for (MatchCallback *MC : Matchers->AllCallbacks) {
- if (EnableCheckProfiling)
- Timer.setBucket(&TimeByBucket[MC->getID()]);
+ if (TimeByBucket)
+ Timer.setBucket(&(*TimeByBucket)[MC->getID()]);
MC->onEndOfTranslationUnit();
}
}
@@ -539,11 +534,10 @@
/// Used by \c matchDispatch() below.
template <typename T, typename MC>
void matchWithoutFilter(const T &Node, const MC &Matchers) {
- const bool EnableCheckProfiling = Options.CheckProfiling.hasValue();
TimeBucketRegion Timer;
for (const auto &MP : Matchers) {
- if (EnableCheckProfiling)
- Timer.setBucket(&TimeByBucket[MP.second->getID()]);
+ if (TimeByBucket)
+ Timer.setBucket(&(*TimeByBucket)[MP.second->getID()]);
BoundNodesTreeBuilder Builder;
if (MP.first.matches(Node, this, &Builder)) {
MatchVisitor Visitor(ActiveASTContext, MP.second);
@@ -561,13 +555,12 @@
if (Filter.empty())
return;
- const bool EnableCheckProfiling = Options.CheckProfiling.hasValue();
TimeBucketRegion Timer;
auto &Matchers = this->Matchers->DeclOrStmt;
for (unsigned short I : Filter) {
auto &MP = Matchers[I];
- if (EnableCheckProfiling)
- Timer.setBucket(&TimeByBucket[MP.second->getID()]);
+ if (TimeByBucket)
+ Timer.setBucket(&(*TimeByBucket)[MP.second->getID()]);
BoundNodesTreeBuilder Builder;
if (MP.first.matchesNoKindCheck(DynNode, this, &Builder)) {
MatchVisitor Visitor(ActiveASTContext, MP.second);
@@ -755,7 +748,7 @@
/// \brief Bucket to record map.
///
/// Used to get the appropriate bucket for each matcher.
- llvm::StringMap<llvm::TimeRecord> TimeByBucket;
+ llvm::StringMap<llvm::TimeRecord> *TimeByBucket = nullptr;
const MatchFinder::MatchersByType *Matchers;
@@ -770,7 +763,6 @@
llvm::DenseMap<ast_type_traits::ASTNodeKind, std::vector<unsigned short>>
MatcherFiltersMap;
- const MatchFinder::MatchFinderOptions &Options;
ASTContext *ActiveASTContext;
// Maps a canonical type to its TypedefDecls.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
