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..cf3497bcec830f5453d9f0ddb440177a2138e8ec 100644
--- a/base/debug/trace_event_impl.cc
+++ b/base/debug/trace_event_impl.cc
@@ -20,6 +20,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 +66,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.
+LazyInstance<ThreadLocalPointer<base::InternedString> >::Leaky
+    g_current_thread_interned_name = LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
@@ -635,31 +638,39 @@ 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);

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

nit: declare/define vars as close as possible to first use.  Move lines 640-642
down to in front of line 652 (or equivalently, move your new block 645-647 up to
around line 639).

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

Done.

 
-    const char* new_name = PlatformThread::GetName();
+
+    if (!g_current_thread_interned_name.Get().Get())

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

nit: not sure about perf impact... but I suspect you should put much if not all
of the above into a local for use in line 652 (and maybe 655).  You're going
through a singleton (locking)... then finding the TLS map.... then getting a
value.  Its seems sad to do this too many times

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

Done.

+      g_current_thread_interned_name.Get().Set(
+          new InternedString(base::kDefaultInternedString));

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

nit: use curlies around any if when you span more than one line.

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

Done.

     // 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.Get().Get())) {
+      const char* new_name = manager->GetInternedStringValue(interned_name);
+      if (new_name && *new_name) {

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

See comments about GetInternedStringValue().  I don't think you want to define
it to return NULL.... and if so... we can avoid checking for it.

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

Done.

+        *(g_current_thread_interned_name.Get().Get()) = 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);
+          }
         }
       }
     }