Add ability to retrieve a thread_name given a thread_id.

base/threading/thread_id_name_manager.cc

Index: base/threading/thread_id_name_manager.cc
diff --git a/base/threading/thread_id_name_manager.cc b/base/threading/thread_id_name_manager.cc
new file mode 100644
index 0000000000000000000000000000000000000000..ea47dbb5746840b3f8b02276693a8402bc0c501a
--- /dev/null
+++ b/base/threading/thread_id_name_manager.cc
@@ -0,0 +1,81 @@
+// 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/threading/thread_id_name_manager.h"
+
+#include <stdlib.h>
+#include <string.h>
+
+#include "base/logging.h"
+#include "base/memory/singleton.h"
+#include "base/string_util.h"
+
+namespace base {
+
+const base::InternedString kDefaultInternedString = 0;

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

nit: avoid using base:: thoughout, since we're in the base namespace.

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

Done.

+
+ThreadIdNameManager::ThreadIdNameManager()
+    : current_interned_name_(1) {
+}
+
+ThreadIdNameManager::~ThreadIdNameManager() {
+}
+
+ThreadIdNameManager* ThreadIdNameManager::GetInstance() {
+  return Singleton<ThreadIdNameManager,
+      LeakySingletonTraits<ThreadIdNameManager> >::get();
+}
+
+void ThreadIdNameManager::SetNameForId(PlatformThreadId id, const char* name) {
+  std::string str_name(name);
+  {

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

nit: curlies not needed, since we return when we release the lock.

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

Done.

+    base::AutoLock locked(lock_);
+
+    if (name_to_interned_name_.count(str_name)) {
+      id_to_interned_name_[id] = name_to_interned_name_[str_name];
+      return;
+    }
+
+    base::InternedString interned_id = current_interned_name_++;
+    id_to_interned_name_[id] = interned_id;

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

nit: code might be cleaner if line 41 and line 36 were only written once. 
Change the sense of the if on line 35 and this will be a more standard pattern:
if (!in cache)
  cache it;
return cached value;

That pattern also has the advantage that the fast (cached) path does one lookup.
 (not sure if perf is an issue here... but it is possibly a nicer pattern).

Similar pattern would apply to line 63 and 76.

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

Done.

+    interned_name_to_name_[interned_id] = str_name;
+    name_to_interned_name_[str_name] = interned_id;
+  }
+}
+
+const char* ThreadIdNameManager::GetNameForId(PlatformThreadId id) {
+  return GetInternedStringValue(GetInternedName(id));
+}
+
+void ThreadIdNameManager::RemoveNameForId(PlatformThreadId id) {
+  if (id == base::kInvalidThreadId)
+    return;
+
+  {

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

nit: remove curlies.

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

Done.

+    base::AutoLock locked(lock_);
+    DCHECK(id_to_interned_name_.count(id));
+    id_to_interned_name_.erase(id);
+  }
+}
+
+base::InternedString ThreadIdNameManager::GetInternedName(PlatformThreadId id) {
+  if (id == base::kInvalidThreadId)
+    return base::kDefaultInternedString;
+
+  base::AutoLock locked(lock_);
+  if (!id_to_interned_name_.count(id))

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

nit: a consistently better pattern is probably to use .find().  If the iterator
is .end(), then it is missing, otherwise you can just return *it.

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

Done.

+    return base::kDefaultInternedString;
+
+  return id_to_interned_name_[id];
+}
+
+const char* ThreadIdNameManager::GetInternedStringValue(
+    base::InternedString name) {
+  base::AutoLock locked(lock_);
+  if (!interned_name_to_name_.count(name))
+    return NULL;

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

At first I thought this was a result of a very invalid argument.  Then I saw
that you get this when you pass in the kDefaultInternedString.

I think the code would be cleaner if you actually had a string for this interned
id of 0, and we never had such error conditions (and when we do, we *know* it is
an error).

As it stands now, such errors get bundled into this line, and not detected.

You could have a call to a static GetDefaultInternedString() which created a ""
string and interned it, and not publicize the constant 0 for instance.

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

Done.

+  return interned_name_to_name_[name].c_str();
+}
+
+}  // namespace base