[clang-tools-extra] r331763 - [clang-tidy] Profile is a per-AST (per-TU) data.
Roman Lebedev via cfe-commits
cfe-commits at lists.llvm.org
Tue May 8 06:14:22 PDT 2018
Author: lebedevri
Date: Tue May 8 06:14:21 2018
New Revision: 331763
URL: http://llvm.org/viewvc/llvm-project?rev=331763&view=rev
Log:
[clang-tidy] Profile is a per-AST (per-TU) data.
Summary:
As discussed in D45931, currently, profiling output of clang-tidy is somewhat not great.
It outputs one profile at the end of the execution, and that profile contains the data
from the last TU that was processed. So if the tool run on multiple TU's, the data is
not accumulated, it is simply discarded.
It would be nice to improve this.
This differential is the first step - make this profiling info per-TU,
and output it after the tool has finished processing each TU.
In particular, when `ClangTidyASTConsumer` destructor runs.
Next step will be to add a CSV (JSON?) printer to store said profiles under user-specified directory prefix.
Reviewers: alexfh, sbenza
Reviewed By: alexfh
Subscribers: Eugene.Zelenko, mgorny, xazax.hun, mgrang, klimek, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D46504
Added:
clang-tools-extra/trunk/clang-tidy/ClangTidyProfiling.cpp
clang-tools-extra/trunk/clang-tidy/ClangTidyProfiling.h
clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp
clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
clang-tools-extra/trunk/clang-tidy/ClangTidy.h
clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp
Modified: clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/CMakeLists.txt?rev=331763&r1=331762&r2=331763&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/CMakeLists.txt Tue May 8 06:14:21 2018
@@ -7,6 +7,7 @@ add_clang_library(clangTidy
ClangTidyModule.cpp
ClangTidyDiagnosticConsumer.cpp
ClangTidyOptions.cpp
+ ClangTidyProfiling.cpp
DEPENDS
ClangSACheckers
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp?rev=331763&r1=331762&r2=331763&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp Tue May 8 06:14:21 2018
@@ -18,6 +18,7 @@
#include "ClangTidy.h"
#include "ClangTidyDiagnosticConsumer.h"
#include "ClangTidyModuleRegistry.h"
+#include "ClangTidyProfiling.h"
#include "clang/AST/ASTConsumer.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
@@ -267,12 +268,17 @@ private:
class ClangTidyASTConsumer : public MultiplexConsumer {
public:
ClangTidyASTConsumer(std::vector<std::unique_ptr<ASTConsumer>> Consumers,
+ std::unique_ptr<ClangTidyProfiling> Profiling,
std::unique_ptr<ast_matchers::MatchFinder> Finder,
std::vector<std::unique_ptr<ClangTidyCheck>> Checks)
- : MultiplexConsumer(std::move(Consumers)), Finder(std::move(Finder)),
+ : MultiplexConsumer(std::move(Consumers)),
+ Profiling(std::move(Profiling)), Finder(std::move(Finder)),
Checks(std::move(Checks)) {}
private:
+ // Destructor order matters! Profiling must be destructed last.
+ // Or at least after Finder.
+ std::unique_ptr<ClangTidyProfiling> Profiling;
std::unique_ptr<ast_matchers::MatchFinder> Finder;
std::vector<std::unique_ptr<ClangTidyCheck>> Checks;
};
@@ -353,8 +359,12 @@ ClangTidyASTConsumerFactory::CreateASTCo
CheckFactories->createChecks(&Context, Checks);
ast_matchers::MatchFinder::MatchFinderOptions FinderOptions;
- if (auto *P = Context.getCheckProfileData())
- FinderOptions.CheckProfiling.emplace(P->Records);
+
+ std::unique_ptr<ClangTidyProfiling> Profiling;
+ if (Context.getEnableProfiling()) {
+ Profiling = llvm::make_unique<ClangTidyProfiling>();
+ FinderOptions.CheckProfiling.emplace(Profiling->Records);
+ }
std::unique_ptr<ast_matchers::MatchFinder> Finder(
new ast_matchers::MatchFinder(std::move(FinderOptions)));
@@ -383,7 +393,8 @@ ClangTidyASTConsumerFactory::CreateASTCo
Consumers.push_back(std::move(AnalysisConsumer));
}
return llvm::make_unique<ClangTidyASTConsumer>(
- std::move(Consumers), std::move(Finder), std::move(Checks));
+ std::move(Consumers), std::move(Profiling), std::move(Finder),
+ std::move(Checks));
}
std::vector<std::string> ClangTidyASTConsumerFactory::getCheckNames() {
@@ -472,7 +483,7 @@ void runClangTidy(clang::tidy::ClangTidy
const CompilationDatabase &Compilations,
ArrayRef<std::string> InputFiles,
llvm::IntrusiveRefCntPtr<vfs::FileSystem> BaseFS,
- ProfileData *Profile) {
+ bool EnableCheckProfile) {
ClangTool Tool(Compilations, InputFiles,
std::make_shared<PCHContainerOperations>(), BaseFS);
@@ -512,8 +523,7 @@ void runClangTidy(clang::tidy::ClangTidy
Tool.appendArgumentsAdjuster(PerFileExtraArgumentsInserter);
Tool.appendArgumentsAdjuster(PluginArgumentsRemover);
- if (Profile)
- Context.setCheckProfileData(Profile);
+ Context.setEnableProfiling(EnableCheckProfile);
ClangTidyDiagnosticConsumer DiagConsumer(Context);
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.h?rev=331763&r1=331762&r2=331763&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.h (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.h Tue May 8 06:14:21 2018
@@ -228,7 +228,7 @@ void runClangTidy(clang::tidy::ClangTidy
const tooling::CompilationDatabase &Compilations,
ArrayRef<std::string> InputFiles,
llvm::IntrusiveRefCntPtr<vfs::FileSystem> BaseFS,
- ProfileData *Profile = nullptr);
+ bool EnableCheckProfile = false);
// FIXME: This interface will need to be significantly extended to be useful.
// FIXME: Implement confidence levels for displaying/fixing errors.
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp?rev=331763&r1=331762&r2=331763&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp Tue May 8 06:14:21 2018
@@ -179,7 +179,7 @@ private:
ClangTidyContext::ClangTidyContext(
std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider)
: DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)),
- Profile(nullptr) {
+ Profile(false) {
// Before the first translation unit we can get errors related to command-line
// parsing, use empty string for the file name in this case.
setCurrentFile("");
@@ -233,7 +233,7 @@ ClangTidyOptions ClangTidyContext::getOp
OptionsProvider->getOptions(File));
}
-void ClangTidyContext::setCheckProfileData(ProfileData *P) { Profile = P; }
+void ClangTidyContext::setEnableProfiling(bool P) { Profile = P; }
bool ClangTidyContext::isCheckEnabled(StringRef CheckName) const {
assert(CheckFilter != nullptr);
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h?rev=331763&r1=331762&r2=331763&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.h Tue May 8 06:14:21 2018
@@ -87,11 +87,6 @@ struct ClangTidyStats {
}
};
-/// \brief Container for clang-tidy profiling data.
-struct ProfileData {
- llvm::StringMap<llvm::TimeRecord> Records;
-};
-
/// \brief Every \c ClangTidyCheck reports errors through a \c DiagnosticsEngine
/// provided by this context.
///
@@ -169,12 +164,9 @@ public:
/// \brief Clears collected errors.
void clearErrors() { Errors.clear(); }
- /// \brief Set the output struct for profile data.
- ///
- /// Setting a non-null pointer here will enable profile collection in
- /// clang-tidy.
- void setCheckProfileData(ProfileData *Profile);
- ProfileData *getCheckProfileData() const { return Profile; }
+ /// \brief Control profile collection in clang-tidy.
+ void setEnableProfiling(bool Profile);
+ bool getEnableProfiling() const { return Profile; }
/// \brief Should be called when starting to process new translation unit.
void setCurrentBuildDirectory(StringRef BuildDirectory) {
@@ -216,7 +208,7 @@ private:
llvm::DenseMap<unsigned, std::string> CheckNamesByDiagnosticID;
- ProfileData *Profile;
+ bool Profile;
};
/// \brief A diagnostic consumer that turns each \c Diagnostic into a
Added: clang-tools-extra/trunk/clang-tidy/ClangTidyProfiling.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyProfiling.cpp?rev=331763&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidyProfiling.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyProfiling.cpp Tue May 8 06:14:21 2018
@@ -0,0 +1,65 @@
+//===--- ClangTidyProfiling.cpp - clang-tidy --------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangTidyProfiling.h"
+#include "llvm/ADT/STLExtras.h"
+
+#define DEBUG_TYPE "clang-tidy-profiling"
+
+namespace clang {
+namespace tidy {
+
+void ClangTidyProfiling::preprocess() {
+ // Convert from a insertion-friendly map to sort-friendly vector.
+ Timers.clear();
+ Timers.reserve(Records.size());
+ for (const auto &P : Records) {
+ Timers.emplace_back(P.getValue(), P.getKey());
+ Total += P.getValue();
+ }
+ assert(Timers.size() == Records.size() && "Size mismatch after processing");
+
+ // We want the measurements to be sorted by decreasing time spent.
+ llvm::sort(Timers.begin(), Timers.end());
+}
+
+void ClangTidyProfiling::printProfileData(llvm::raw_ostream &OS) const {
+ std::string Line = "===" + std::string(73, '-') + "===\n";
+ OS << Line;
+
+ if (Total.getUserTime())
+ OS << " ---User Time---";
+ if (Total.getSystemTime())
+ OS << " --System Time--";
+ if (Total.getProcessTime())
+ OS << " --User+System--";
+ OS << " ---Wall Time---";
+ if (Total.getMemUsed())
+ OS << " ---Mem---";
+ OS << " --- Name ---\n";
+
+ // Loop through all of the timing data, printing it out.
+ for (auto I = Timers.rbegin(), E = Timers.rend(); I != E; ++I) {
+ I->first.print(Total, OS);
+ OS << I->second << '\n';
+ }
+
+ Total.print(Total, OS);
+ OS << "Total\n";
+ OS << Line << "\n";
+ OS.flush();
+}
+
+ClangTidyProfiling::~ClangTidyProfiling() {
+ preprocess();
+ printProfileData(llvm::errs());
+}
+
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/ClangTidyProfiling.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyProfiling.h?rev=331763&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidyProfiling.h (added)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyProfiling.h Tue May 8 06:14:21 2018
@@ -0,0 +1,41 @@
+//===--- ClangTidyProfiling.h - clang-tidy ----------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYPROFILING_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYPROFILING_H
+
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Timer.h"
+#include "llvm/Support/raw_ostream.h"
+#include <utility>
+#include <vector>
+
+namespace clang {
+namespace tidy {
+
+class ClangTidyProfiling {
+ // Time is first to allow for sorting by it.
+ std::vector<std::pair<llvm::TimeRecord, llvm::StringRef>> Timers;
+ llvm::TimeRecord Total;
+
+ void preprocess();
+
+ void printProfileData(llvm::raw_ostream &OS) const;
+
+public:
+ llvm::StringMap<llvm::TimeRecord> Records;
+
+ ~ClangTidyProfiling();
+};
+
+} // end namespace tidy
+} // end namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYPROFILING_H
Modified: clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp?rev=331763&r1=331762&r2=331763&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp Tue May 8 06:14:21 2018
@@ -236,45 +236,6 @@ static void printStats(const ClangTidySt
}
}
-static void printProfileData(const ProfileData &Profile,
- llvm::raw_ostream &OS) {
- // Time is first to allow for sorting by it.
- std::vector<std::pair<llvm::TimeRecord, StringRef>> Timers;
- TimeRecord Total;
-
- for (const auto &P : Profile.Records) {
- Timers.emplace_back(P.getValue(), P.getKey());
- Total += P.getValue();
- }
-
- std::sort(Timers.begin(), Timers.end());
-
- std::string Line = "===" + std::string(73, '-') + "===\n";
- OS << Line;
-
- if (Total.getUserTime())
- OS << " ---User Time---";
- if (Total.getSystemTime())
- OS << " --System Time--";
- if (Total.getProcessTime())
- OS << " --User+System--";
- OS << " ---Wall Time---";
- if (Total.getMemUsed())
- OS << " ---Mem---";
- OS << " --- Name ---\n";
-
- // Loop through all of the timing data, printing it out.
- for (auto I = Timers.rbegin(), E = Timers.rend(); I != E; ++I) {
- I->first.print(Total, OS);
- OS << I->second << '\n';
- }
-
- Total.print(Total, OS);
- OS << "Total\n";
- OS << Line << "\n";
- OS.flush();
-}
-
static std::unique_ptr<ClangTidyOptionsProvider> createOptionsProvider(
llvm::IntrusiveRefCntPtr<vfs::FileSystem> FS) {
ClangTidyGlobalOptions GlobalOptions;
@@ -424,7 +385,6 @@ static int clangTidyMain(int argc, const
llvm::cl::PrintHelpMessage(/*Hidden=*/false, /*Categorized=*/true);
return 1;
}
- ProfileData Profile;
llvm::InitializeAllTargetInfos();
llvm::InitializeAllTargetMCs();
@@ -432,7 +392,7 @@ static int clangTidyMain(int argc, const
ClangTidyContext Context(std::move(OwningOptionsProvider));
runClangTidy(Context, OptionsParser.getCompilations(), PathList, BaseFS,
- EnableCheckProfile ? &Profile : nullptr);
+ EnableCheckProfile);
ArrayRef<ClangTidyError> Errors = Context.getErrors();
bool FoundErrors = llvm::find_if(Errors, [](const ClangTidyError &E) {
return E.DiagLevel == ClangTidyError::Error;
@@ -464,9 +424,6 @@ static int clangTidyMain(int argc, const
"Fixes have NOT been applied.\n\n";
}
- if (EnableCheckProfile)
- printProfileData(Profile, llvm::errs());
-
if (WErrorCount) {
if (!Quiet) {
StringRef Plural = WErrorCount == 1 ? "" : "s";
Added: clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp?rev=331763&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-one-tu.cpp Tue May 8 06:14:21 2018
@@ -0,0 +1,18 @@
+// RUN: clang-tidy -enable-check-profile -checks='-*,readability-function-size' %s 2>&1 | FileCheck --match-full-lines -implicit-check-not='{{warning:|error:}}' %s
+
+// CHECK: ===-------------------------------------------------------------------------===
+// CHECK-NEXT: {{.*}} --- Name ---
+// CHECK-NEXT: {{.*}} readability-function-size
+// CHECK-NEXT: {{.*}} Total
+// CHECK-NEXT: ===-------------------------------------------------------------------------===
+
+// CHECK-NOT: ===-------------------------------------------------------------------------===
+// CHECK-NOT: {{.*}} --- Name ---
+// CHECK-NOT: {{.*}} readability-function-size
+// CHECK-NOT: {{.*}} Total
+// CHECK-NOT: ===-------------------------------------------------------------------------===
+
+class A {
+ A() {}
+ ~A() {}
+};
Added: clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp?rev=331763&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/clang-tidy-enable-check-profile-two-tu.cpp Tue May 8 06:14:21 2018
@@ -0,0 +1,24 @@
+// RUN: clang-tidy -enable-check-profile -checks='-*,readability-function-size' %s %s 2>&1 | FileCheck --match-full-lines -implicit-check-not='{{warning:|error:}}' %s
+
+// CHECK: ===-------------------------------------------------------------------------===
+// CHECK-NEXT: {{.*}} --- Name ---
+// CHECK-NEXT: {{.*}} readability-function-size
+// CHECK-NEXT: {{.*}} Total
+// CHECK-NEXT: ===-------------------------------------------------------------------------===
+
+// CHECK: ===-------------------------------------------------------------------------===
+// CHECK-NEXT: {{.*}} --- Name ---
+// CHECK-NEXT: {{.*}} readability-function-size
+// CHECK-NEXT: {{.*}} Total
+// CHECK-NEXT: ===-------------------------------------------------------------------------===
+
+// CHECK-NOT: ===-------------------------------------------------------------------------===
+// CHECK-NOT: {{.*}} --- Name ---
+// CHECK-NOT: {{.*}} readability-function-size
+// CHECK-NOT: {{.*}} Total
+// CHECK-NOT: ===-------------------------------------------------------------------------===
+
+class A {
+ A() {}
+ ~A() {}
+};
More information about the cfe-commits
mailing list