[tracing] Adding task information to heap profiler

base/tracked_objects.cc

Index: base/tracked_objects.cc
diff --git a/base/tracked_objects.cc b/base/tracked_objects.cc
index d24cedf159241d58a92909650de374f187dd85ae..10c3ed8b894e3edd90c0b3a40f3705ff5a192987 100644
--- a/base/tracked_objects.cc
+++ b/base/tracked_objects.cc
@@ -16,6 +16,7 @@
 #include "base/process/process_handle.h"
 #include "base/strings/stringprintf.h"
 #include "base/third_party/valgrind/memcheck.h"
+#include "base/trace_event/heap_profiler_allocation_context_tracker.h"
 #include "base/tracking_info.h"
 #include "build/build_config.h"
 
@@ -362,6 +363,14 @@ void ThreadData::InitializeThreadContext(const std::string& suggested_name) {
     return;  // Browser tests instigate this.
   current_thread_data = new ThreadData(suggested_name);
   tls_index_.Set(current_thread_data);
+
+  // Add thread name to pseudo stack frame of heap profiler. The name added here
+  // is valid for the lifetime of the process since ThreadData is not destroyed
+  // even if thread is dead.

[Primiano Tucci (use gerrit), 2016/03/15 16:44:33]

Bad news:
I took a look to this and figured out that tracked_objects is a bit dodgy, in
the sense that the thread name is leaked, but not in tests (which might cause
flakiness).
On the good side, I figured out that thread_id_name_manager.cc actually leaks
the string with the thread name (See leaked_str in
ThreadIdNameManager::SetName).

Good news:
So I think you can achieve this by doing no change in base (outside of
trace_event) at all.
Ideally (*there is a catch, see below) in the AllocationContexTracker CTOR, you
can just do:

::AllocationContexTracker() {
 PushPseudoStackFrame(ThreadIdNameManager::GetName(GetCurrentThreadID()))
}

Bad news:
If you do this you will cause a re-entrancy in the heap profiler that will
deadlock. The pattern will be:
the first time you use the ContextTracker -> Ctor() -> PushPseudoStackFrame will
call malloc -> malloc will be intercepted -> you are back in ContextTracker ->
BOOM


Good news:
After my CL (crrev.com/1675183006) lands I think you can put that piece of code
in ::InitializeForCurrentThread() which has malloc re-entrancy protection

https://www.youtube.com/watch?v=Krbl911ZPBA

[ssid, 2016/03/17 00:11:47]

I think I should continue with good news and bad news here :P

Calling base::ThreadIdNameManager::GetName causes deadlock because the first
allocation that happens (which also initializes tracker) in the threads is from
base::ThreadIdNameManager::RegisterThread which holds the lock needed to get
name.

I am setting the name from ThreadIdNameManager::SetName. Ideally I should be
doing it form PlatformThread::Setname which is a static function that is called
in a thread to set it's own name.
Where as ThreadIdNameManager::SetName is a member of singleton
ThreadIdNameManager which can be called from any thread, which will reset the
tls of another thread if it happens. But the PlatformThread::SetName is defined
for each platform so I should change 6-7 files for that. Instead I have now put
it in ThreadIdNameManager because the only place that calls
ThreadIdNameManager::SetName is PlatformThread::SetName which is always called
in the same thread. So, I prefer changing just ThreadIdNameManager instead of
all the files. If you think it is safer to set it from PlatformThread then I can
change.

+  if (base::trace_event::AllocationContextTracker::capture_enabled()) {
+    base::trace_event::AllocationContextTracker::SetThreadName(
+        tracked_objects::ThreadData::Get()->thread_name().c_str());
+  }
 }
 
 // static