Record Chrome trace events in tcmalloc heap profiles

third_party/tcmalloc/chromium/src/heap-profiler.cc

Index: third_party/tcmalloc/chromium/src/heap-profiler.cc
diff --git a/third_party/tcmalloc/chromium/src/heap-profiler.cc b/third_party/tcmalloc/chromium/src/heap-profiler.cc
index 0b07a5e8662da230e481839dcb5f62b521f9359d..a6f1e47f5a5e821c701d2d1b758e58e2abbce7f9 100644
--- a/third_party/tcmalloc/chromium/src/heap-profiler.cc
+++ b/third_party/tcmalloc/chromium/src/heap-profiler.cc
@@ -222,6 +222,9 @@ static int64 last_dump_time = 0;      // The time of the last dump
 static HeapProfileTable* heap_profile = NULL;  // the heap profile table
 static DeepHeapProfile* deep_profile = NULL;  // deep memory profiler
 
+// Callback an appplication can use to generate its own "stacks".
+static PseudoStackGenerator pseudo_stack_generator = NULL;
+
 //----------------------------------------------------------------------
 // Profile generation
 //----------------------------------------------------------------------
@@ -382,6 +385,22 @@ static void RecordAlloc(const void* ptr, size_t bytes, int skip_count) {
   }
 }
 
+// Record an allocation in the profile. This function is performance critical
+// so it does not attempt to share code with RecordAlloc() above.

[Dai Mikurube (NOT FULLTIME), 2013/07/01 05:47:42]

Is it really performance critical? I don't think it affects performance even if
an if-clause is added while I don't see the actual numbers. Also, in my
understanding, it's not always on (only when tracing is on). I don't think it's
worth to add almost duplicated new functions for it. Do you have numbers that
show performance impact?

The HeapProfiler is almost always on (|is_on|) when (PseudoStack)RecordAlloc is
called.  heap_profile->RecordAlloc is enough heavy to hide the performance
impact since it always computes a hash value from a (maybe pseudo-)stack trace
in GetBucket() and operates a hash table in AddressMap.

In addition, we may be able to do it without an if-clause.

[James Cook, 2013/07/01 23:51:20]

I benchmarked it both with and without a branch (introduced with ?:) -- there is
no measurable difference. Thanks for suggesting I benchmark it.  I consolidated
the functions.

[Dai Mikurube (NOT FULLTIME), 2013/07/02 01:39:06]

Thank you for working on it!

JFYI, I'm not confident, but I think we may not need any branch if you
initialize |pseudo_stack_generator| with HeapProfileTable::GetCallerStackTrace
with using the same function signature.

+static void PseudoStackRecordAlloc(const void* ptr,
+                                   size_t bytes,
+                                   int skip_count) {
+  // Take the stack trace outside the critical section.
+  void* stack[HeapProfileTable::kMaxStackDepth];
+  // Generate our pseudo-stack by via callback into the client code.
+  int depth = (*pseudo_stack_generator)(stack);
+  SpinLockHolder l(&heap_lock);
+  if (is_on) {
+    heap_profile->RecordAlloc(ptr, bytes, depth, stack);
+    MaybeDumpProfileLocked();
+  }
+}
+
 // Record a deallocation in the profile.
 static void RecordFree(const void* ptr) {
   SpinLockHolder l(&heap_lock);
@@ -405,6 +424,11 @@ void DeleteHook(const void* ptr) {
   if (ptr != NULL) RecordFree(ptr);
 }
 
+// static
+void PseudoStackNewHook(const void* ptr, size_t size) {
+  if (ptr != NULL) PseudoStackRecordAlloc(ptr, size, 0);
+}
+
 // TODO(jandrews): Re-enable stack tracing
 #ifdef TODO_REENABLE_STACK_TRACING
 static void RawInfoStackDumper(const char* message, void*) {
@@ -538,16 +562,39 @@ extern "C" void HeapProfilerStart(const char* prefix) {
 
   if (FLAGS_only_mmap_profile == false) {
     // Now set the hooks that capture new/delete and malloc/free.
-    RAW_CHECK(MallocHook::AddNewHook(&NewHook), "");
+    if (pseudo_stack_generator) {
+      RAW_CHECK(MallocHook::AddNewHook(&PseudoStackNewHook), "");
+    } else {
+      RAW_CHECK(MallocHook::AddNewHook(&NewHook), "");
+    }
     RAW_CHECK(MallocHook::AddDeleteHook(&DeleteHook), "");
   }
 
-  // Copy filename prefix
-  RAW_DCHECK(filename_prefix == NULL, "");
-  const int prefix_length = strlen(prefix);
-  filename_prefix = reinterpret_cast<char*>(ProfilerMalloc(prefix_length + 1));
-  memcpy(filename_prefix, prefix, prefix_length);
-  filename_prefix[prefix_length] = '\0';
+  // Copy filename prefix if provided.
+  if (prefix) {
+    RAW_DCHECK(filename_prefix == NULL, "");
+    const int prefix_length = strlen(prefix);
+    filename_prefix =
+        reinterpret_cast<char*>(ProfilerMalloc(prefix_length + 1));
+    memcpy(filename_prefix, prefix, prefix_length);
+    filename_prefix[prefix_length] = '\0';
+  }
+}
+
+extern "C" void HeapProfilerWithPseudoStackStart(
+    PseudoStackGenerator callback) {
+  {
+    // Ensure the callback is set before allocations can be recorded.
+    SpinLockHolder l(&heap_lock);
+    pseudo_stack_generator = callback;
+  }
+  HeapProfilerStart(NULL);
+  {
+    // The data from /proc/self/maps is not required for pseudo-stack profiles
+    // and increases the size of the profile dumps significantly.
+    SpinLockHolder l(&heap_lock);
+    heap_profile->DisableProfileSelfMaps();
+  }
 }
 
 extern "C" void IterateAllocatedObjects(AddressVisitor visitor, void* data) {
@@ -570,7 +617,11 @@ extern "C" void HeapProfilerStop() {
 
   if (FLAGS_only_mmap_profile == false) {
     // Unset our new/delete hooks, checking they were set:
-    RAW_CHECK(MallocHook::RemoveNewHook(&NewHook), "");
+    if (pseudo_stack_generator) {
+      RAW_CHECK(MallocHook::RemoveNewHook(&PseudoStackNewHook), "");
+    } else {
+      RAW_CHECK(MallocHook::RemoveNewHook(&NewHook), "");
+    }
     RAW_CHECK(MallocHook::RemoveDeleteHook(&DeleteHook), "");
   }
   if (FLAGS_mmap_log) {