Add ability to retrieve a thread_name given a thread_id.

base/debug/trace_event_impl.cc

Index: base/debug/trace_event_impl.cc
diff --git a/base/debug/trace_event_impl.cc b/base/debug/trace_event_impl.cc
index 07679d1131250fb101401191d02434dc6e59e8b8..5abe48e430bb72e6982f59fe54c0f1cf1d408ca4 100644
--- a/base/debug/trace_event_impl.cc
+++ b/base/debug/trace_event_impl.cc
@@ -10,7 +10,6 @@
 #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"
@@ -20,6 +19,7 @@
 #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"
@@ -65,9 +65,11 @@ 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
 
@@ -635,31 +637,35 @@ void TraceLog::AddTraceEvent(char phase,
       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);
+          }
         }
       }
     }