[NFC] Add PrintOnExit parameter to a llvm::TimerGroup

[necto]

Clean up AnalysisConsumer code from the timer-related branches that are not used most of the time, and move this logic to Timer.cpp, which is a more relevant place and allows for a cleaner implementation.

[llvmbot]

@llvm/pr-subscribers-llvm-support
@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Author: Arseniy Zaostrovnykh (necto)

Changes

Clean up AnalysisConsumer code from the timer-related branches that are not used most of the time, and move this logic to Timer.cpp, which is a more relevant place and allows for a cleaner implementation.

---

Full diff: https://github.com/llvm/llvm-project/pull/164407.diff

3 Files Affected:

• (modified) clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp (+4-23)
• (modified) llvm/include/llvm/Support/Timer.h (+4-2)
• (modified) llvm/lib/Support/Timer.cpp (+9-8)

diff --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp index 871400e29362f..e5811d0d4bf1e 100644 --- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp +++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp @@ -128,7 +128,6 @@ class AnalysisConsumer : public AnalysisASTConsumer, std::unique_ptr<llvm::Timer> SyntaxCheckTimer; std::unique_ptr<llvm::Timer> ExprEngineTimer; std::unique_ptr<llvm::Timer> BugReporterTimer; - bool ShouldClearTimersToPreventDisplayingThem; /// The information about analyzed functions shared throughout the /// translation unit. @@ -149,7 +148,10 @@ class AnalysisConsumer : public AnalysisASTConsumer, if (Opts.AnalyzerDisplayProgress || Opts.PrintStats || Opts.ShouldSerializeStats || !Opts.DumpEntryPointStatsToCSV.empty()) { AnalyzerTimers = std::make_unique<llvm::TimerGroup>( - "analyzer", "Analyzer timers"); + "analyzer", "Analyzer timers", + /*PrintOnExit=*/ + (Opts.AnalyzerDisplayProgress || Opts.PrintStats || + Opts.ShouldSerializeStats)); SyntaxCheckTimer = std::make_unique<llvm::Timer>( "syntaxchecks", "Syntax-based analysis time", *AnalyzerTimers); ExprEngineTimer = std::make_unique<llvm::Timer>( @@ -159,12 +161,6 @@ class AnalysisConsumer : public AnalysisASTConsumer, *AnalyzerTimers); } - // Avoid displaying the timers created above in case we only want to record - // per-entry-point stats. - ShouldClearTimersToPreventDisplayingThem = !Opts.AnalyzerDisplayProgress && - !Opts.PrintStats && - !Opts.ShouldSerializeStats; - if (Opts.PrintStats || Opts.ShouldSerializeStats) { llvm::EnableStatistics(/* DoPrintOnExit= */ false); } @@ -287,9 +283,6 @@ class AnalysisConsumer : public AnalysisASTConsumer, checkerMgr->runCheckersOnASTDecl(D, *Mgr, *RecVisitorBR); if (SyntaxCheckTimer) SyntaxCheckTimer->stopTimer(); - if (AnalyzerTimers && ShouldClearTimersToPreventDisplayingThem) { - AnalyzerTimers->clear(); - } } return true; } @@ -583,9 +576,6 @@ void AnalysisConsumer::runAnalysisOnTranslationUnit(ASTContext &C) { checkerMgr->runCheckersOnASTDecl(TU, *Mgr, BR); if (SyntaxCheckTimer) SyntaxCheckTimer->stopTimer(); - if (AnalyzerTimers && ShouldClearTimersToPreventDisplayingThem) { - AnalyzerTimers->clear(); - } // Run the AST-only checks using the order in which functions are defined. // If inlining is not turned on, use the simplest function order for path @@ -765,9 +755,6 @@ void AnalysisConsumer::HandleCode(Decl *D, AnalysisMode Mode, FunctionSummaries.findOrInsertSummary(D)->second.SyntaxRunningTime = std::lround(CheckerEndTime.getWallTime() * 1000); DisplayTime(CheckerEndTime); - if (AnalyzerTimers && ShouldClearTimersToPreventDisplayingThem) { - AnalyzerTimers->clear(); - } } } @@ -830,9 +817,6 @@ void AnalysisConsumer::RunPathSensitiveChecks(Decl *D, PathRunningTime.set(static_cast<unsigned>( std::lround(ExprEngineEndTime.getWallTime() * 1000))); DisplayTime(ExprEngineEndTime); - if (AnalyzerTimers && ShouldClearTimersToPreventDisplayingThem) { - AnalyzerTimers->clear(); - } } if (!Mgr->options.DumpExplodedGraphTo.empty()) @@ -843,9 +827,6 @@ void AnalysisConsumer::RunPathSensitiveChecks(Decl *D, Eng.ViewGraph(Mgr->options.TrimGraph); flushReports(BugReporterTimer.get(), Eng.getBugReporter()); - if (AnalyzerTimers && ShouldClearTimersToPreventDisplayingThem) { - AnalyzerTimers->clear(); - } } //===----------------------------------------------------------------------===// diff --git a/llvm/include/llvm/Support/Timer.h b/llvm/include/llvm/Support/Timer.h index a4ed712577582..089d1b7b40dc4 100644 --- a/llvm/include/llvm/Support/Timer.h +++ b/llvm/include/llvm/Support/Timer.h @@ -203,6 +203,7 @@ class TimerGroup { std::string Description; Timer *FirstTimer = nullptr; ///< First timer in the group. std::vector<PrintRecord> TimersToPrint; + bool PrintOnExit; TimerGroup **Prev; ///< Pointer to Next field of previous timergroup in list. TimerGroup *Next; ///< Pointer to next timergroup in list. @@ -211,10 +212,11 @@ class TimerGroup { friend class TimerGlobals; explicit TimerGroup(StringRef Name, StringRef Description, - sys::SmartMutex<true> &lock); + sys::SmartMutex<true> &lock, bool PrintOnExit); public: - LLVM_ABI explicit TimerGroup(StringRef Name, StringRef Description); + LLVM_ABI explicit TimerGroup(StringRef Name, StringRef Description, + bool PrintOnExit = true); LLVM_ABI explicit TimerGroup(StringRef Name, StringRef Description, const StringMap<TimeRecord> &Records); diff --git a/llvm/lib/Support/Timer.cpp b/llvm/lib/Support/Timer.cpp index 67483ba9dd655..a9455c70f536f 100644 --- a/llvm/lib/Support/Timer.cpp +++ b/llvm/lib/Support/Timer.cpp @@ -240,7 +240,7 @@ class Name2PairMap { getGroupEntry(StringRef GroupName, StringRef GroupDescription) { std::pair<TimerGroup *, Name2TimerMap> &GroupEntry = Map[GroupName]; if (!GroupEntry.first) - GroupEntry.first = new TimerGroup(GroupName, GroupDescription); + GroupEntry.first = new TimerGroup(GroupName, GroupDescription, /*PrintOnExit=*/true); return GroupEntry; } @@ -270,9 +270,10 @@ TimerGroup &NamedRegionTimer::getNamedTimerGroup(StringRef GroupName, static TimerGroup *TimerGroupList = nullptr; TimerGroup::TimerGroup(StringRef Name, StringRef Description, - sys::SmartMutex<true> &lock) + sys::SmartMutex<true> &lock, bool PrintOnExit) : Name(Name.begin(), Name.end()), - Description(Description.begin(), Description.end()) { + Description(Description.begin(), Description.end()), + PrintOnExit(PrintOnExit) { // Add the group to TimerGroupList. sys::SmartScopedLock<true> L(lock); if (TimerGroupList) @@ -282,12 +283,12 @@ TimerGroup::TimerGroup(StringRef Name, StringRef Description, TimerGroupList = this; } -TimerGroup::TimerGroup(StringRef Name, StringRef Description) - : TimerGroup(Name, Description, timerLock()) {} +TimerGroup::TimerGroup(StringRef Name, StringRef Description, bool PrintOnExit) + : TimerGroup(Name, Description, timerLock(), PrintOnExit) {} TimerGroup::TimerGroup(StringRef Name, StringRef Description, const StringMap<TimeRecord> &Records) - : TimerGroup(Name, Description) { + : TimerGroup(Name, Description, /*PrintOnExit=*/false) { TimersToPrint.reserve(Records.size()); for (const auto &P : Records) TimersToPrint.emplace_back(P.getValue(), std::string(P.getKey()), @@ -301,7 +302,7 @@ TimerGroup::~TimerGroup() { while (FirstTimer) removeTimer(*FirstTimer); - if (!TimersToPrint.empty()) { + if (!TimersToPrint.empty() && PrintOnExit) { std::unique_ptr<raw_ostream> OutStream = CreateInfoOutputFile(); PrintQueuedTimers(*OutStream); } @@ -530,7 +531,7 @@ class llvm::TimerGlobals { sys::SmartMutex<true> TimerLock; TimerGroup DefaultTimerGroup{"misc", "Miscellaneous Ungrouped Timers", - TimerLock}; + TimerLock, /*PrintOnExit=*/true}; SignpostEmitter Signposts; // Order of these members and initialization below is important. For example 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[steakhal]

Should't this overload also forward the PrintOnExit?

[necto]

I opted for minimal change, since I do not use this constructor in the context where PrintOnExit can be false. but you are right, the default value should be true, my bad

[necto]

4831dc7 Set PrintOnExit to true by default in the other constructor