Add ability to retrieve a thread_name given a thread_id.

base/debug/trace_event_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/debug/trace_event_impl.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/debug/leak_annotations.h"
 #include "base/debug/trace_event.h"
 #include "base/format_macros.h"
-#include "base/lazy_instance.h"
 #include "base/memory/singleton.h"
 #include "base/process_util.h"
 #include "base/stl_util.h"
 #include "base/stringprintf.h"
 #include "base/string_tokenizer.h"
 #include "base/string_util.h"
 #include "base/sys_info.h"
 #include "base/third_party/dynamic_annotations/dynamic_annotations.h"
 #include "base/threading/platform_thread.h"
+#include "base/threading/thread_id_name_manager.h"
 #include "base/threading/thread_local.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 
 #if defined(OS_WIN)
 #include "base/debug/trace_event_win.h"
 #endif
 
 class DeleteTraceLogForTesting {
  public:
@@ skipping 25 matching lines @@
   "tracing categories exhausted; must increase TRACE_EVENT_MAX_CATEGORIES",
   "__metadata",
 };
 // The enabled flag is char instead of bool so that the API can be used from C.
 unsigned char g_category_enabled[TRACE_EVENT_MAX_CATEGORIES] = { 0 };
 const int g_category_already_shutdown = 0;
 const int g_category_categories_exhausted = 1;
 const int g_category_metadata = 2;
 int g_category_index = 3; // skip initial 3 categories
 
-// The most-recently captured name of the current thread
-LazyInstance<ThreadLocalPointer<const char> >::Leaky
-    g_current_thread_name = LAZY_INSTANCE_INITIALIZER;
+// The name of the current thread. This is used to decide if the current
+// thread name has changed. We combine all the seen thread names into the
+// output name for the thread.
+base::InternedString g_current_thread_interned_name =

[jar (doing other things), 2013/01/14 19:27:57]

I was expecting such a thing to be in TLS, and it's not clear how this is held
in a global.

[dsinclair, 2013/01/14 22:26:09]

Thanks for pointing that out, lost the ThreadLocal at some point when I was
working on this. It's back now.

+    base::kDefaultInternedString;
 
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 //
 // TraceEvent
 //
 ////////////////////////////////////////////////////////////////////////////////
 
 namespace {
@@ skipping 547 matching lines @@
   TimeTicks now = TimeTicks::NowFromSystemTraceTime() - time_offset_;
   NotificationHelper notifier(this);
   {
     AutoLock lock(lock_);
     if (*category_enabled != CATEGORY_ENABLED)
       return;
     if (logged_events_.size() >= kTraceEventBufferSize)
       return;
 
     int thread_id = static_cast<int>(PlatformThread::CurrentId());
+    ThreadIdNameManager* manager = ThreadIdNameManager::GetInstance();
+    InternedString interned_name = manager->GetInternedName(thread_id);
 
-    const char* new_name = PlatformThread::GetName();
     // Check if the thread name has been set or changed since the previous
     // call (if any), but don't bother if the new name is empty. Note this will
     // not detect a thread name change within the same char* buffer address: we
     // favor common case performance over corner case correctness.
-    if (new_name != g_current_thread_name.Get().Get() &&
-        new_name && *new_name) {
-      g_current_thread_name.Get().Set(new_name);
-      base::hash_map<int, std::string>::iterator existing_name =
-          thread_names_.find(thread_id);
-      if (existing_name == thread_names_.end()) {
-        // This is a new thread id, and a new name.
-        thread_names_[thread_id] = new_name;
-      } else {
-        // This is a thread id that we've seen before, but potentially with a
-        // new name.
-        std::vector<base::StringPiece> existing_names;
-        Tokenize(existing_name->second, ",", &existing_names);
-        bool found = std::find(existing_names.begin(),
-                               existing_names.end(),
-                               new_name) != existing_names.end();
-        if (!found) {
-          existing_name->second.push_back(',');
-          existing_name->second.append(new_name);
+    if (interned_name != g_current_thread_interned_name) {

[jar (doing other things), 2013/01/14 19:27:57]

How does this work without racing access to the global?

[dsinclair, 2013/01/14 22:26:09]

Moved the storage to a thread local. There is an AutoLock lock around this
specific region of code, so should fix any races.

[jar (doing other things), 2013/01/17 23:18:39]

Shouldn't there be a test that would break if we didn't have TLS?  Can you add
one?

On 2013/01/14 22:26:09, dsinclair wrote:

[dsinclair, 2013/01/21 19:14:18]

I'm not exactly sure how to write that test. Any suggestions?

This code is actually back to the form it was in prior to me making any changes.
With the other changes suggested I was able to do the name manager without
modifying the trace code.


On 2013/01/17 23:18:39, jar wrote:

+      const char* new_name = manager->GetInternedStringValue(interned_name);
+      if (new_name && *new_name) {
+        g_current_thread_interned_name = interned_name;
+
+        base::hash_map<int, std::string>::iterator existing_name =
+            thread_names_.find(thread_id);
+        if (existing_name == thread_names_.end()) {
+          // This is a new thread id, and a new name.
+          thread_names_[thread_id] = new_name;
+        } else {
+          // This is a thread id that we've seen before, but potentially with a
+          // new name.
+          std::vector<base::StringPiece> existing_names;
+          Tokenize(existing_name->second, ",", &existing_names);
+          bool found = std::find(existing_names.begin(),
+                                 existing_names.end(),
+                                 new_name) != existing_names.end();
+          if (!found) {
+            existing_name->second.push_back(',');
+            existing_name->second.append(new_name);
+          }
         }
       }
     }
 
     if (flags & TRACE_EVENT_FLAG_MANGLE_ID)
       id ^= process_id_hash_;
 
     logged_events_.push_back(
         TraceEvent(thread_id,
                    now, phase, category_enabled, name, id,
@@ skipping 107 matching lines @@
   unsigned long long pid = static_cast<unsigned long long>(process_id_);
   process_id_hash_ = (offset_basis ^ pid) * fnv_prime;
 }
 
 void TraceLog::SetTimeOffset(TimeDelta offset) {
   time_offset_ = offset;
 }
 
 }  // namespace debug
 }  // namespace base