Remove Timer::Initialize routine

[labath]

Summary

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.

[zturner]

source/Core/Timer.cpp

68 ↗

(On Diff #46380)

What did we decide about the C++11 thread_local keyword? I know it's been blocked on MSVC compiler support, but now that we're basically up to VS2015 on Windows...

[clayborg]

Looks fine. Zach: I don't believe that Mac supports the thread local keywords correctly yet, but I can check on that. We would need to get the OK from multiple people (MacOSX, Windows, FreeBSD, Linux and Android) before we could think of switching...

[labath]

We are still using VS2013 at the moment, but we have started to investigate the possibility of migrating to 2015. I can report back when I know more, but I would leave the thread_local keyword out for now.

lldb/trunk/include/lldb/Core/Timer.h

public:
     //--------------------------------------------------------------
     Timer(const char *category, const char *format, ...)  __attribute__ ((format (printf, 3, 4)));
 
     //--------------------------------------------------------------
     /// Destructor
     //--------------------------------------------------------------
     ~Timer();
 
-    static void
-    Initialize ();
-
     void
     Dump ();
 
     static void
     SetDisplayDepth (uint32_t depth);
     
     static void
     SetQuiet (bool value);
protected:
     TimeValue m_total_start;
     TimeValue m_timer_start;
     uint64_t m_total_ticks; // Total running time for this timer including when other timers below this are running
     uint64_t m_timer_ticks; // Ticks for this timer that do not include when other timers below this one are running
 
     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();
     DISALLOW_COPY_AND_ASSIGN (Timer);
 };
     
 class IntervalTimer
 {

lldb/trunk/source/Core/Timer.cpp

struct TimerStack
         uint32_t m_depth;
         std::vector<Timer*> m_stack;
     };
 } // end of anonymous namespace
 
 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()
 {
     static Mutex g_category_mutex(Mutex::eMutexTypeNormal);
     return g_category_mutex;
 }
 
 static TimerCategoryMap &
 GetCategoryMap()
 {
     static TimerCategoryMap g_category_map;
     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)
     {
         Host::ThreadLocalStorageSet(g_key, new TimerStack);
         timer_stack = Host::ThreadLocalStorageGet(g_key);
     }
     return (TimerStack *)timer_stack;
 }
 
 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 (),
     m_timer_start (),
     m_total_ticks (0),
     m_timer_ticks (0)
 {
     TimerStack *stack = GetTimerStackForCurrentThread ();
     if (!stack)
         return;
 
     if (stack->m_depth++ < g_display_depth)
     {
         if (g_quiet == false)
         {
             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;
         m_timer_start = start_time;
 
         if (!stack->m_stack.empty())
             stack->m_stack.back()->ChildStarted (start_time);
         stack->m_stack.push_back(this);
if (m_total_start.IsValid())
         const uint64_t total_nsec_uint = GetTotalElapsedNanoSeconds();
         const uint64_t timer_nsec_uint = GetTimerElapsedNanoSeconds();
         const double total_nsec = total_nsec_uint;
         const double timer_nsec = timer_nsec_uint;
 
         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.
         Mutex::Locker locker (GetCategoryMutex());
         TimerCategoryMap &category_map = GetCategoryMap();
         category_map[m_category] += timer_nsec_uint;
     }
     if (stack->m_depth > 0)

lldb/trunk/source/Initialization/SystemInitializerCommon.cpp

if (disable_crash_dialog_var && llvm::StringRef(disable_crash_dialog_var).equals_lower("true"))
         _CrtSetReportFile(_CRT_ASSERT, _CRTDBG_FILE_STDERR);
         _CrtSetReportFile(_CRT_WARN, _CRTDBG_FILE_STDERR);
         _CrtSetReportFile(_CRT_ERROR, _CRTDBG_FILE_STDERR);
     }
 #endif
 
     Log::Initialize();
     HostInfo::Initialize();
-    Timer::Initialize();
     Timer scoped_timer(__PRETTY_FUNCTION__, __PRETTY_FUNCTION__);
 
     llvm::install_fatal_error_handler(fatal_error_handler, 0);
 
     process_gdb_remote::ProcessGDBRemoteLog::Initialize();
 
     // Initialize plug-ins
     ClangASTContext::Initialize();