[Support] Reduce globaal variable overhead after #121663 #122429

MaskRay · 2025-01-10T07:11:02Z

Construct frequently-accessed TimerLock/DefaultTimerGroup early to
reduce overhead.
Rename aquireDefaultGroup to acquireTimerGlobals and restore
ManagedStatic::claim. https://reviews.llvm.org/D76099
Drop mtg::. We use internal linkage, so mtg:: is unneeded and might
mislead users. In addition, llvm/ code almost never introduces a named
namespace not in llvm::. Drop mtg::.
Replace some unique_ptr with optional to reduce overhead.
Switch to functionName().
Simplify llvm::initTimerOptions and TimerGroup::constructForStatistics()

Created using spr 1.3.5-bogner

llvmbot · 2025-01-10T07:11:35Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-llvm-support

Author: Fangrui Song (MaskRay)

Changes

We use internal linkage, so mtg:: is unneeded and might mislead users.
In addition, llvm/ code almost never introduces a named namespace not in
llvm::.

While here, switch to functionName() and simplify llvm::initTimerOptions.

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

1 Files Affected:

(modified) llvm/lib/Support/Timer.cpp (+52-63)

diff --git a/llvm/lib/Support/Timer.cpp b/llvm/lib/Support/Timer.cpp
index 3f0926ae0f3cbe..9cf6cf058c1086 100644
--- a/llvm/lib/Support/Timer.cpp
+++ b/llvm/lib/Support/Timer.cpp
@@ -39,7 +39,7 @@
 using namespace llvm;
 
 //===----------------------------------------------------------------------===//
-// Forward declarations for Managed Timer Globals (mtg) getters.
+// Forward declarations for Managed Timer Globals getters.
 //
 // Globals have been placed at the end of the file to restrict direct
 // access. Use of getters also has the benefit of making it a bit more explicit
@@ -49,29 +49,21 @@ namespace {
 class Name2PairMap;
 }
 
-namespace mtg {
-static std::string &LibSupportInfoOutputFilename();
-static const std::string &InfoOutputFilename();
-static bool TrackSpace();
-static bool SortTimers();
-static SignpostEmitter &Signposts();
-static sys::SmartMutex<true> &TimerLock();
-static TimerGroup &DefaultTimerGroup();
+static std::string &libSupportInfoOutputFilename();
+static bool trackSpace();
+static bool sortTimers();
+static SignpostEmitter &signposts();
+static sys::SmartMutex<true> &timerLock();
+static TimerGroup &defaultTimerGroup();
 static TimerGroup *claimDefaultTimerGroup();
-static Name2PairMap &NamedGroupedTimers();
-} // namespace mtg
+static Name2PairMap &namedGroupedTimers();
 
 //===----------------------------------------------------------------------===//
 //
 //===----------------------------------------------------------------------===//
-void llvm::initTimerOptions() {
-  mtg::TrackSpace();
-  mtg::InfoOutputFilename();
-  mtg::SortTimers();
-}
 
 std::unique_ptr<raw_ostream> llvm::CreateInfoOutputFile() {
-  const std::string &OutputFilename = mtg::LibSupportInfoOutputFilename();
+  const std::string &OutputFilename = libSupportInfoOutputFilename();
   if (OutputFilename.empty())
     return std::make_unique<raw_fd_ostream>(2, false); // stderr.
   if (OutputFilename == "-")
@@ -97,7 +89,7 @@ std::unique_ptr<raw_ostream> llvm::CreateInfoOutputFile() {
 //===----------------------------------------------------------------------===//
 
 void Timer::init(StringRef TimerName, StringRef TimerDescription) {
-  init(TimerName, TimerDescription, mtg::DefaultTimerGroup());
+  init(TimerName, TimerDescription, defaultTimerGroup());
 }
 
 void Timer::init(StringRef TimerName, StringRef TimerDescription,
@@ -116,7 +108,7 @@ Timer::~Timer() {
 }
 
 static inline size_t getMemUsage() {
-  if (!mtg::TrackSpace())
+  if (!trackSpace())
     return 0;
   return sys::Process::GetMallocUsage();
 }
@@ -157,7 +149,7 @@ TimeRecord TimeRecord::getCurrentTime(bool Start) {
 void Timer::startTimer() {
   assert(!Running && "Cannot start a running timer");
   Running = Triggered = true;
-  mtg::Signposts().startInterval(this, getName());
+  signposts().startInterval(this, getName());
   StartTime = TimeRecord::getCurrentTime(true);
 }
 
@@ -166,7 +158,7 @@ void Timer::stopTimer() {
   Running = false;
   Time += TimeRecord::getCurrentTime(false);
   Time -= StartTime;
-  mtg::Signposts().endInterval(this, getName());
+  signposts().endInterval(this, getName());
 }
 
 void Timer::clear() {
@@ -218,7 +210,7 @@ class Name2PairMap {
 
   Timer &get(StringRef Name, StringRef Description, StringRef GroupName,
              StringRef GroupDescription) {
-    sys::SmartScopedLock<true> L(mtg::TimerLock());
+    sys::SmartScopedLock<true> L(timerLock());
 
     std::pair<TimerGroup*, Name2TimerMap> &GroupEntry = Map[GroupName];
 
@@ -237,17 +229,17 @@ class Name2PairMap {
 NamedRegionTimer::NamedRegionTimer(StringRef Name, StringRef Description,
                                    StringRef GroupName,
                                    StringRef GroupDescription, bool Enabled)
-    : TimeRegion(!Enabled ? nullptr
-                          : &mtg::NamedGroupedTimers().get(Name, Description,
-                                                           GroupName,
-                                                           GroupDescription)) {}
+    : TimeRegion(!Enabled
+                     ? nullptr
+                     : &namedGroupedTimers().get(Name, Description, GroupName,
+                                                 GroupDescription)) {}
 
 //===----------------------------------------------------------------------===//
 //   TimerGroup Implementation
 //===----------------------------------------------------------------------===//
 
 /// This is the global list of TimerGroups, maintained by the TimerGroup
-/// ctor/dtor and is protected by the TimerLock lock.
+/// ctor/dtor and is protected by the timerLock lock.
 static TimerGroup *TimerGroupList = nullptr;
 
 TimerGroup::TimerGroup(StringRef Name, StringRef Description,
@@ -264,7 +256,7 @@ TimerGroup::TimerGroup(StringRef Name, StringRef Description,
 }
 
 TimerGroup::TimerGroup(StringRef Name, StringRef Description)
-    : TimerGroup(Name, Description, mtg::TimerLock()) {}
+    : TimerGroup(Name, Description, timerLock()) {}
 
 TimerGroup::TimerGroup(StringRef Name, StringRef Description,
                        const StringMap<TimeRecord> &Records)
@@ -283,7 +275,7 @@ TimerGroup::~TimerGroup() {
     removeTimer(*FirstTimer);
 
   // Remove the group from the TimerGroupList.
-  sys::SmartScopedLock<true> L(mtg::TimerLock());
+  sys::SmartScopedLock<true> L(timerLock());
   *Prev = Next;
   if (Next)
     Next->Prev = Prev;
@@ -291,7 +283,7 @@ TimerGroup::~TimerGroup() {
 
 
 void TimerGroup::removeTimer(Timer &T) {
-  sys::SmartScopedLock<true> L(mtg::TimerLock());
+  sys::SmartScopedLock<true> L(timerLock());
 
   // If the timer was started, move its data to TimersToPrint.
   if (T.hasTriggered())
@@ -314,7 +306,7 @@ void TimerGroup::removeTimer(Timer &T) {
 }
 
 void TimerGroup::addTimer(Timer &T) {
-  sys::SmartScopedLock<true> L(mtg::TimerLock());
+  sys::SmartScopedLock<true> L(timerLock());
 
   // Add the timer to our list.
   if (FirstTimer)
@@ -326,7 +318,7 @@ void TimerGroup::addTimer(Timer &T) {
 
 void TimerGroup::PrintQueuedTimers(raw_ostream &OS) {
   // Perhaps sort the timers in descending order by amount of time taken.
-  if (mtg::SortTimers())
+  if (sortTimers())
     llvm::sort(TimersToPrint);
 
   TimeRecord Total;
@@ -344,7 +336,7 @@ void TimerGroup::PrintQueuedTimers(raw_ostream &OS) {
   // If this is not an collection of ungrouped times, print the total time.
   // Ungrouped timers don't really make sense to add up.  We still print the
   // TOTAL line to make the percentages make sense.
-  if (this != &mtg::DefaultTimerGroup())
+  if (this != &defaultTimerGroup())
     OS << format("  Total Execution Time: %5.4f seconds (%5.4f wall clock)\n",
                  Total.getProcessTime(), Total.getWallTime());
   OS << '\n';
@@ -396,7 +388,7 @@ void TimerGroup::prepareToPrintList(bool ResetTime) {
 void TimerGroup::print(raw_ostream &OS, bool ResetAfterPrint) {
   {
     // After preparing the timers we can free the lock
-    sys::SmartScopedLock<true> L(mtg::TimerLock());
+    sys::SmartScopedLock<true> L(timerLock());
     prepareToPrintList(ResetAfterPrint);
   }
 
@@ -406,20 +398,20 @@ void TimerGroup::print(raw_ostream &OS, bool ResetAfterPrint) {
 }
 
 void TimerGroup::clear() {
-  sys::SmartScopedLock<true> L(mtg::TimerLock());
+  sys::SmartScopedLock<true> L(timerLock());
   for (Timer *T = FirstTimer; T; T = T->Next)
     T->clear();
 }
 
 void TimerGroup::printAll(raw_ostream &OS) {
-  sys::SmartScopedLock<true> L(mtg::TimerLock());
+  sys::SmartScopedLock<true> L(timerLock());
 
   for (TimerGroup *TG = TimerGroupList; TG; TG = TG->Next)
     TG->print(OS);
 }
 
 void TimerGroup::clearAll() {
-  sys::SmartScopedLock<true> L(mtg::TimerLock());
+  sys::SmartScopedLock<true> L(timerLock());
   for (TimerGroup *TG = TimerGroupList; TG; TG = TG->Next)
     TG->clear();
 }
@@ -436,7 +428,7 @@ void TimerGroup::printJSONValue(raw_ostream &OS, const PrintRecord &R,
 }
 
 const char *TimerGroup::printJSONValues(raw_ostream &OS, const char *delim) {
-  sys::SmartScopedLock<true> L(mtg::TimerLock());
+  sys::SmartScopedLock<true> L(timerLock());
 
   prepareToPrintList(false);
   for (const PrintRecord &R : TimersToPrint) {
@@ -463,19 +455,19 @@ const char *TimerGroup::printJSONValues(raw_ostream &OS, const char *delim) {
 }
 
 const char *TimerGroup::printAllJSONValues(raw_ostream &OS, const char *delim) {
-  sys::SmartScopedLock<true> L(mtg::TimerLock());
+  sys::SmartScopedLock<true> L(timerLock());
   for (TimerGroup *TG = TimerGroupList; TG; TG = TG->Next)
     delim = TG->printJSONValues(OS, delim);
   return delim;
 }
 
 void TimerGroup::constructForStatistics() {
-  mtg::LibSupportInfoOutputFilename();
-  mtg::NamedGroupedTimers();
+  libSupportInfoOutputFilename();
+  namedGroupedTimers();
 }
 
 std::unique_ptr<TimerGroup> TimerGroup::aquireDefaultGroup() {
-  return std::unique_ptr<TimerGroup>(mtg::claimDefaultTimerGroup());
+  return std::unique_ptr<TimerGroup>(claimDefaultTimerGroup());
 }
 
 //===----------------------------------------------------------------------===//
@@ -505,7 +497,7 @@ class llvm::TimerGlobals {
 
 private:
   // Order of these members and initialization below is important. For example
-  // the DefaultTimerGroup uses the TimerLock. Most of these also depend on the
+  // the defaultTimerGroup uses the timerLock. Most of these also depend on the
   // options above.
   std::once_flag InitDeferredFlag;
   std::unique_ptr<SignpostEmitter> SignpostsPtr;
@@ -524,15 +516,15 @@ class llvm::TimerGlobals {
   }
 
 public:
-  SignpostEmitter &Signposts() { return *initDeferred().SignpostsPtr; }
-  sys::SmartMutex<true> &TimerLock() { return *initDeferred().TimerLockPtr; }
-  TimerGroup &DefaultTimerGroup() {
+  SignpostEmitter &signposts() { return *initDeferred().SignpostsPtr; }
+  sys::SmartMutex<true> &timerLock() { return *initDeferred().TimerLockPtr; }
+  TimerGroup &defaultTimerGroup() {
     return *initDeferred().DefaultTimerGroupPtr;
   }
   TimerGroup *claimDefaultTimerGroup() {
     return initDeferred().DefaultTimerGroupPtr.release();
   }
-  Name2PairMap &NamedGroupedTimers() {
+  Name2PairMap &namedGroupedTimers() {
     return *initDeferred().NamedGroupedTimersPtr;
   }
 
@@ -556,26 +548,23 @@ class llvm::TimerGlobals {
 
 static ManagedStatic<TimerGlobals> ManagedTimerGlobals;
 
-static std::string &mtg::LibSupportInfoOutputFilename() {
+static std::string &libSupportInfoOutputFilename() {
   return ManagedTimerGlobals->LibSupportInfoOutputFilename;
 }
-static const std::string &mtg::InfoOutputFilename() {
-  return ManagedTimerGlobals->InfoOutputFilename.getValue();
-}
-static bool mtg::TrackSpace() { return ManagedTimerGlobals->TrackSpace; }
-static bool mtg::SortTimers() { return ManagedTimerGlobals->SortTimers; }
-static SignpostEmitter &mtg::Signposts() {
-  return ManagedTimerGlobals->Signposts();
+static bool trackSpace() { return ManagedTimerGlobals->TrackSpace; }
+static bool sortTimers() { return ManagedTimerGlobals->SortTimers; }
+static SignpostEmitter &signposts() { return ManagedTimerGlobals->signposts(); }
+static sys::SmartMutex<true> &timerLock() {
+  return ManagedTimerGlobals->timerLock();
 }
-static sys::SmartMutex<true> &mtg::TimerLock() {
-  return ManagedTimerGlobals->TimerLock();
+static TimerGroup &defaultTimerGroup() {
+  return ManagedTimerGlobals->defaultTimerGroup();
 }
-static TimerGroup &mtg::DefaultTimerGroup() {
-  return ManagedTimerGlobals->DefaultTimerGroup();
-}
-static TimerGroup *mtg::claimDefaultTimerGroup() {
+static TimerGroup *claimDefaultTimerGroup() {
   return ManagedTimerGlobals->claimDefaultTimerGroup();
 }
-static Name2PairMap &mtg::NamedGroupedTimers() {
-  return ManagedTimerGlobals->NamedGroupedTimers();
+static Name2PairMap &namedGroupedTimers() {
+  return ManagedTimerGlobals->namedGroupedTimers();
 }
+
+void llvm::initTimerOptions() { *ManagedTimerGlobals; }

Created using spr 1.3.5-bogner

macurtis-amd

LGTM

Created using spr 1.3.5-bogner

* Construct frequently-accessed TimerLock/DefaultTimerGroup early to reduce overhead. * Rename `aquireDefaultGroup` to `acquireTimerGlobals` and restore ManagedStatic::claim. https://reviews.llvm.org/D76099 * Drop mtg::. We use internal linkage, so mtg:: is unneeded and might mislead users. In addition, llvm/ code almost never introduces a named namespace not in llvm::. Drop mtg::. * Replace some unique_ptr with optional to reduce overhead. * Switch to `functionName()`. * Simplify `llvm::initTimerOptions` and `TimerGroup::constructForStatistics()` Pull Request: llvm/llvm-project#122429

* Construct frequently-accessed TimerLock/DefaultTimerGroup early to reduce overhead. * Rename `aquireDefaultGroup` to `acquireTimerGlobals` and restore ManagedStatic::claim. https://reviews.llvm.org/D76099 * Drop mtg::. We use internal linkage, so mtg:: is unneeded and might mislead users. In addition, llvm/ code almost never introduces a named namespace not in llvm::. Drop mtg::. * Replace some unique_ptr with optional to reduce overhead. * Switch to `functionName()`. * Simplify `llvm::initTimerOptions` and `TimerGroup::constructForStatistics()` Pull Request: llvm#122429

[𝘀𝗽𝗿] initial version

320ab02

Created using spr 1.3.5-bogner

llvmbot added the llvm:support label Jan 10, 2025

MaskRay requested review from macurtis-amd and ronlieb January 10, 2025 07:11

MaskRay mentioned this pull request Jan 10, 2025

[llvm][NFC] Rework Timer.cpp globals to ensure valid lifetimes #121663

Merged

.

cc49050

Created using spr 1.3.5-bogner

macurtis-amd approved these changes Jan 10, 2025

View reviewed changes

repair #121663

366d63f

Created using spr 1.3.5-bogner

MaskRay changed the title ~~[Support] Drop mtg:: from #121663 to Timer.cpp~~ [Support] Reduce globaal variable overhead after #121663 Jan 10, 2025

llvmbot added the clang Clang issues not falling into any other category label Jan 10, 2025

MaskRay added 2 commits January 10, 2025 09:00

remove defined-once member functions that are only used once

f270d40

Created using spr 1.3.5-bogner

.

5d2ad37

Created using spr 1.3.5-bogner

MaskRay merged commit af4d76d into main Jan 11, 2025
8 checks passed

MaskRay deleted the users/MaskRay/spr/support-drop-mtg-from-121663-to-timercpp branch January 11, 2025 01:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Support] Reduce globaal variable overhead after #121663 #122429

[Support] Reduce globaal variable overhead after #121663 #122429

Uh oh!

MaskRay commented Jan 10, 2025 •

edited

Loading

Uh oh!

llvmbot commented Jan 10, 2025 •

edited

Loading

Uh oh!

macurtis-amd left a comment

Uh oh!

Uh oh!

Uh oh!

[Support] Reduce globaal variable overhead after #121663 #122429

[Support] Reduce globaal variable overhead after #121663 #122429

Uh oh!

Conversation

MaskRay commented Jan 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jan 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

macurtis-amd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

MaskRay commented Jan 10, 2025 •

edited

Loading

llvmbot commented Jan 10, 2025 •

edited

Loading