[Lldb-commits] [PATCH] D16722: Remove Timer::Initialize routine

[Pavel Labath via lldb-commits]

labath created this revision.
labath added reviewers: clayborg, zturner, tberghammer.
labath added a subscriber: lldb-commits.

I've run into an issue when running unit tests, where the underlying problem 
turned out to be
that we were creating Timer objects (through several layers of indirection) 
without calling
Timer::Initialize. Since Timer's thread-local storage was not properly 
initialized, we were
overwriting gtest's own thread-local storage, causing test failures.

Instead of requiring that every test calls Timer::Initialize(), I remove the 
function altogether:
The thread-local storage can be initialized on-demand, and the g_file variable 
initialized to
stdout and never changed, so I have simply removed it.

http://reviews.llvm.org/D16722

Files:
  include/lldb/Core/Timer.h
  source/Core/Timer.cpp
  source/Initialization/SystemInitializerCommon.cpp


Index: source/Initialization/SystemInitializerCommon.cpp
===================================================================
--- source/Initialization/SystemInitializerCommon.cpp
+++ source/Initialization/SystemInitializerCommon.cpp
@@ -97,7 +97,6 @@
 
     Log::Initialize();
     HostInfo::Initialize();
-    Timer::Initialize();
     Timer scoped_timer(__PRETTY_FUNCTION__, __PRETTY_FUNCTION__);
 
     llvm::install_fatal_error_handler(fatal_error_handler, 0);
Index: source/Core/Timer.cpp
===================================================================
--- source/Core/Timer.cpp
+++ source/Core/Timer.cpp
@@ -40,9 +40,7 @@
 std::atomic<bool> Timer::g_quiet(true);
 std::atomic<unsigned> Timer::g_display_depth(0);
 std::mutex Timer::g_file_mutex;
-FILE* Timer::g_file = nullptr;
 
-static lldb::thread_key_t g_key;
 
 static Mutex &
 GetCategoryMutex()
@@ -58,10 +56,17 @@
     return g_category_map;
 }
 
+static void
+ThreadSpecificCleanup(void *p)
+{
+    delete static_cast<TimerStack *>(p);
+}
 
 static TimerStack *
 GetTimerStackForCurrentThread ()
 {
+    static lldb::thread_key_t g_key = Host::ThreadLocalStorageCreate(ThreadSpecificCleanup);
+
     void *timer_stack = Host::ThreadLocalStorageGet(g_key);
     if (timer_stack == NULL)
     {
@@ -72,24 +77,11 @@
 }
 
 void
-ThreadSpecificCleanup (void *p)
-{
-    delete (TimerStack *)p;
-}
-
-void
 Timer::SetQuiet (bool value)
 {
     g_quiet = value;
 }
 
-void
-Timer::Initialize ()
-{
-    Timer::g_file = stdout;
-    g_key = Host::ThreadLocalStorageCreate(ThreadSpecificCleanup);
-}
-
 Timer::Timer (const char *category, const char *format, ...) :
     m_category (category),
     m_total_start (),
@@ -108,15 +100,15 @@
             std::lock_guard<std::mutex> lock(g_file_mutex);
 
             // Indent
-            ::fprintf (g_file, "%*s", stack->m_depth * TIMER_INDENT_AMOUNT, "");
+            ::fprintf(stdout, "%*s", stack->m_depth * TIMER_INDENT_AMOUNT, "");
             // Print formatted string
             va_list args;
             va_start (args, format);
-            ::vfprintf (g_file, format, args);
+            ::vfprintf(stdout, format, args);
             va_end (args);
 
             // Newline
-            ::fprintf (g_file, "\n");
+            ::fprintf(stdout, "\n");
         }
         TimeValue start_time(TimeValue::Now());
         m_total_start = start_time;
@@ -161,11 +153,8 @@
         if (g_quiet == false)
         {
             std::lock_guard<std::mutex> lock(g_file_mutex);
-            ::fprintf (g_file,
-                       "%*s%.9f sec (%.9f sec)\n",
-                       (stack->m_depth - 1) *TIMER_INDENT_AMOUNT, "",
-                       total_nsec / 1000000000.0,
-                       timer_nsec / 1000000000.0);
+            ::fprintf(stdout, "%*s%.9f sec (%.9f sec)\n", (stack->m_depth - 1) * TIMER_INDENT_AMOUNT, "",
+                      total_nsec / 1000000000.0, timer_nsec / 1000000000.0);
         }
 
         // Keep total results for each category so we can dump results.
Index: include/lldb/Core/Timer.h
===================================================================
--- include/lldb/Core/Timer.h
+++ include/lldb/Core/Timer.h
@@ -50,9 +50,6 @@
     //--------------------------------------------------------------
     ~Timer();
 
-    static void
-    Initialize ();
-
     void
     Dump ();
 
@@ -90,7 +87,6 @@
     static std::atomic<bool> g_quiet;
     static std::atomic<unsigned> g_display_depth;
     static std::mutex g_file_mutex;
-    static FILE* g_file;
 
 private:
     Timer();

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits