[PATCH] D28367: Correct default TimerGroup singleton to use magic-statics so that it gets cleaned up

[Erich Keane via Phabricator via llvm-commits]

erichkeane created this revision.
erichkeane added reviewers: craig.topper, lattner, aaron.ballman.
erichkeane added a subscriber: llvm-commits.

When running Valgrind, I discovered that the string in the default TimerGroup (created in getDefaultTimerGroup) gets 'possibly lost', it is never deleted in the current implementation.  The TimerGroup object allocates a string, but it is stored as a pointer in getDefaultTimerGroup(), so it is never cleaned up.

This patch uses a magic-static to maintain the thread-safe behavior, however stores the value on the stack so that it gets cleaned up afterwards.

A few things:
1- The mutex that was previously created will be taken inside the DefaultTimerGroup's constructor, so there shouldn't be any concurrency problems that didn't previously exist.
2- I realize that this fix doesn't terribly matter too much, since it is cleaning stuff up right before program termination, but I thought it is a pretty minor cost for code correctness and removing the error from valgrind (reducing noise!)


https://reviews.llvm.org/D28367

Files:
  lib/Support/Timer.cpp


Index: lib/Support/Timer.cpp
===================================================================
--- lib/Support/Timer.cpp
+++ lib/Support/Timer.cpp
@@ -72,22 +72,9 @@
   return llvm::make_unique<raw_fd_ostream>(2, false); // stderr.
 }
 
-
-static TimerGroup *DefaultTimerGroup = nullptr;
 static TimerGroup *getDefaultTimerGroup() {
-  TimerGroup *tmp = DefaultTimerGroup;
-  sys::MemoryFence();
-  if (tmp) return tmp;
-
-  sys::SmartScopedLock<true> Lock(*TimerLock);
-  tmp = DefaultTimerGroup;
-  if (!tmp) {
-    tmp = new TimerGroup("misc", "Miscellaneous Ungrouped Timers");
-    sys::MemoryFence();
-    DefaultTimerGroup = tmp;
-  }
-
-  return tmp;
+  static TimerGroup DefaultTimerGroup("misc", "Miscellaneous Ungrouped Timers");
+  return &DefaultTimerGroup;
 }
 
 //===----------------------------------------------------------------------===//
@@ -309,7 +296,7 @@
   // 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 != DefaultTimerGroup)
+  if (this != getDefaultTimerGroup())
     OS << format("  Total Execution Time: %5.4f seconds (%5.4f wall clock)\n",
                  Total.getProcessTime(), Total.getWallTime());
   OS << '\n';


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D28367.83270.patch
Type: text/x-patch
Size: 1320 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170105/f7eacd6c/attachment.bin>