[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