tracing: add dump provider for malloc heap profiler

base/trace_event/malloc_dump_provider.cc

 // Copyright 2015 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/trace_event/malloc_dump_provider.h"
 
 #include <stddef.h>
 
 #include "base/allocator/allocator_extension.h"
+#include "base/allocator/allocator_shim.h"
+#include "base/allocator/features.h"
+#include "base/trace_event/heap_profiler_allocation_context.h"
+#include "base/trace_event/heap_profiler_allocation_context_tracker.h"
+#include "base/trace_event/heap_profiler_allocation_register.h"
+#include "base/trace_event/heap_profiler_heap_dump_writer.h"
 #include "base/trace_event/process_memory_dump.h"
+#include "base/trace_event/trace_event_argument.h"
 #include "build/build_config.h"
 
 #if defined(OS_MACOSX)
 #include <malloc/malloc.h>
 #else
 #include <malloc.h>
 #endif
 
 namespace base {
 namespace trace_event {
 
+#if BUILDFLAG(USE_EXPERIMENTAL_ALLOCATOR_SHIM)
+namespace {
+
+using allocator::AllocatorDispatch;
+
+void* HookAlloc(const AllocatorDispatch* self, size_t size) {
+  const AllocatorDispatch* const next = self->next;
+  void* ptr = next->alloc_function(next, size);
+  if (ptr)
+    MallocDumpProvider::GetInstance()->InsertAllocation(ptr, size);
+  return ptr;
+}
+
+void* HookZeroInitAlloc(const AllocatorDispatch* self, size_t n, size_t size) {
+  const AllocatorDispatch* const next = self->next;
+  void* ptr = next->alloc_zero_initialized_function(next, n, size);
+  if (ptr)
+    MallocDumpProvider::GetInstance()->InsertAllocation(ptr, n * size);
+  return ptr;
+}
+
+void* HookllocAligned(const AllocatorDispatch* self,
+                      size_t alignment,
+                      size_t size) {
+  const AllocatorDispatch* const next = self->next;
+  void* ptr = next->alloc_aligned_function(next, alignment, size);
+  if (ptr)
+    MallocDumpProvider::GetInstance()->InsertAllocation(ptr, size);
+  return ptr;
+}
+
+void* HookRealloc(const AllocatorDispatch* self, void* address, size_t size) {
+  const AllocatorDispatch* const next = self->next;
+  void* ptr = next->realloc_function(next, address, size);
+  MallocDumpProvider::GetInstance()->RemoveAllocation(address);
+  if (size > 0)  // realloc(size == 0) means free().
+    MallocDumpProvider::GetInstance()->InsertAllocation(ptr, size);
+  return ptr;
+}
+
+void HookFree(const AllocatorDispatch* self, void* address) {
+  if (address)
+    MallocDumpProvider::GetInstance()->RemoveAllocation(address);
+  const AllocatorDispatch* const next = self->next;
+  next->free_function(next, address);
+}
+
+AllocatorDispatch g_allocator_hooks = {
+    &HookAlloc,         /* alloc_function */
+    &HookZeroInitAlloc, /* alloc_zero_initialized_function */
+    &HookllocAligned,   /* alloc_aligned_function */
+    &HookRealloc,       /* realloc_function */
+    &HookFree,          /* free_function */
+    nullptr,            /* next */
+};
+
+}  // namespace
+#endif  // BUILDFLAG(USE_EXPERIMENTAL_ALLOCATOR_SHIM)
+
 // static
 const char MallocDumpProvider::kAllocatedObjects[] = "malloc/allocated_objects";
 
 // static
 MallocDumpProvider* MallocDumpProvider::GetInstance() {
   return Singleton<MallocDumpProvider,
                    LeakySingletonTraits<MallocDumpProvider>>::get();
 }
 
-MallocDumpProvider::MallocDumpProvider() {}
+MallocDumpProvider::MallocDumpProvider()
+    : heap_profiler_enabled_(false), tid_dumping_heap_(kInvalidThreadId) {}
 
 MallocDumpProvider::~MallocDumpProvider() {}
 
 // Called at trace dump point time. Creates a snapshot the memory counters for
 // the current process.
 bool MallocDumpProvider::OnMemoryDump(const MemoryDumpArgs& args,
                                       ProcessMemoryDump* pmd) {
   size_t total_virtual_size = 0;
   size_t resident_size = 0;
   size_t allocated_objects_size = 0;
@@ skipping 47 matching lines @@
     // Explicitly specify why is extra memory resident. In tcmalloc it accounts
     // for free lists and caches. In mac and ios it accounts for the
     // fragmentation and metadata.
     MemoryAllocatorDump* other_dump =
         pmd->CreateAllocatorDump("malloc/metadata_fragmentation_caches");
     other_dump->AddScalar(MemoryAllocatorDump::kNameSize,
                           MemoryAllocatorDump::kUnitsBytes,
                           resident_size - allocated_objects_size);
   }
 
+  // Heap profiler dumps.
+  if (!heap_profiler_enabled_)
+    return true;
+
+  // The dumps of the heap profiler should be created only when heap profiling
+  // was enabled (--enable-heap-profiling) AND a DETAILED dump is requested.
+  // However, when enabled, the overhead of the heap profiler should be always
+  // reported (w.r.t LIGHT vs DETAILED), to avoid oscillations in the malloc()

[petrcermak, 2016/03/11 11:09:23]

nit: s/w.r.t/w.r.t./ and s/vs/vs./

[petrcermak, 2016/03/11 11:09:24]

supernit: I don't think there should be a comma before "to" here.

[petrcermak, 2016/03/11 11:09:24]

supernit: I think you should s/malloc()/malloc/ or even s/malloc()/'malloc'/
because 'malloc' is then shown in the tracing UI.

[petrcermak, 2016/03/11 11:09:24]

What do you mean by "w.r.t LIGHT vs DETAILED"? Don't you mean "even in LIGHT
dumps"?

[Primiano Tucci (use gerrit), 2016/03/11 13:57:57]

Done.

[Primiano Tucci (use gerrit), 2016/03/11 13:57:57]

Ok reworded this.

[Primiano Tucci (use gerrit), 2016/03/11 13:57:57]

Done.

[Primiano Tucci (use gerrit), 2016/03/11 13:57:57]

Done.

+  // total.
+
+  tid_dumping_heap_ = PlatformThread::CurrentId();
+  // At this point the Insert/RemoveAllocation hooks will ignore this thread.
+  // Enclosing all the temporariy data structures in a scope, so that the heap
+  // profiler does not see unabalanced malloc/free calls from these containers.
+  {
+    TraceEventMemoryOverhead overhead;
+    hash_map<AllocationContext, size_t> bytes_by_context;
+    {
+      AutoLock lock(allocation_register_lock_);
+      if (allocation_register_) {
+        if (args.level_of_detail == MemoryDumpLevelOfDetail::DETAILED) {
+          for (const auto& alloc_size : *allocation_register_)
+            bytes_by_context[alloc_size.context] += alloc_size.size;
+        }
+        allocation_register_->EstimateTraceMemoryOverhead(&overhead);
+      }
+    }  // lock(allocation_register_lock_)
+
+    if (!bytes_by_context.empty()) {
+      scoped_ptr<TracedValue> heap_dump = ExportHeapDump(
+          bytes_by_context, pmd->session_state()->stack_frame_deduplicator(),
+          pmd->session_state()->type_name_deduplicator());
+      pmd->AddHeapDump("malloc", std::move(heap_dump));
+    }
+    overhead.DumpInto("tracing/heap_profiler_malloc", pmd);

[petrcermak, 2016/03/11 11:09:24]

thought: Wouldn't "tracing/malloc_heap_profiler" be better (easier to find in
the UI)?

[Primiano Tucci (use gerrit), 2016/03/11 13:57:57]

In the beginning it was like that, but I made it consistent with the
partition_alloc and blink_gc, which today already use heap_profiler_XXX

[petrcermak, 2016/03/11 14:26:31]

Acknowledged.

+  }
+  tid_dumping_heap_ = kInvalidThreadId;
+
   return true;
 }
 
+void MallocDumpProvider::OnHeapProfilingEnabled(bool enabled) {
+#if BUILDFLAG(USE_EXPERIMENTAL_ALLOCATOR_SHIM)
+  if (enabled) {
+    {
+      AutoLock lock(allocation_register_lock_);
+      allocation_register_.reset(new AllocationRegister());
+    }
+    allocator::InsertAllocatorDispatch(&g_allocator_hooks);
+  } else {
+    AutoLock lock(allocation_register_lock_);
+    allocation_register_.reset();
+    // Inser/RemoveAllocation below will no-op if the register is torn down.

[petrcermak, 2016/03/11 11:09:24]

s/Inser/Insert/

[Primiano Tucci (use gerrit), 2016/03/11 13:57:57]

Done.

+  }
+#endif
+  heap_profiler_enabled_ = enabled;
+}
+
+void MallocDumpProvider::InsertAllocation(void* address, size_t size) {
+  if (tid_dumping_heap_ != kInvalidThreadId &&

[petrcermak, 2016/03/11 11:09:24]

Can it ever happen that PlatformThread::CurrentId() == kInvalidThreadID? If not,
the first case is unnecessary. Also applies to line 250.

[Primiano Tucci (use gerrit), 2016/03/11 13:57:57]

Heh I knew that you would have commented this. You are conceptually right, but
there is a perf reason for doing this.
CurrentId() is slow on some platforms, as it might end up invoking a syscall (if
the libc implementation is dumb).
In most of the cases, I expect to not be in the reentrant situation here, in
which case the first check will short-circuit the if and prevent the CurrentID()
call at all.
Good spot btw :)

[petrcermak, 2016/03/11 14:26:31]

Acknowledged.

+      tid_dumping_heap_ == PlatformThread::CurrentId())
+    return;
+  // AllocationContextTracker initialization is the only thing here that can
+  // cause re-entrancy.
+  switch (AllocationContextTracker::GetStateForCurrentThread()) {
+    case AllocationContextTracker::kInitialized:
+      break;
+    case AllocationContextTracker::kNotInitialized:
+      AllocationContextTracker::InitializeForCurrentThread();
+      return;
+    case AllocationContextTracker::kInitializing:
+      return;
+  }
+
+  AllocationContext context = AllocationContextTracker::GetContextSnapshot();
+
+  AutoLock lock(allocation_register_lock_);

[petrcermak, 2016/03/11 11:09:24]

Can't the compiler move this definition to the top of the method (i.e. shouldn't
lines 242-246 be in a block)? Also applies to lines 253-256.

[Primiano Tucci (use gerrit), 2016/03/11 13:57:57]

nope. The compiler is not allowed to reorder instructions unless it is sure that
there is no side effect, which is not the case for Lock (because uses atomics,
and more in general is not inlined and defined into another translation unit).
The reason why sometimes you see Lock in a scope, is if you want the effect of
the lock to end before the end of the function, e.g.,

{
   Lock;
   do_something_locked();
}
do_something_unlocked();

+  if (!allocation_register_)
+    return;
+
+  allocation_register_->Insert(address, size, context);
+}
+
+void MallocDumpProvider::RemoveAllocation(void* address) {
+  if (tid_dumping_heap_ != kInvalidThreadId &&
+      tid_dumping_heap_ == PlatformThread::CurrentId())
+    return;
+  AutoLock lock(allocation_register_lock_);

[petrcermak, 2016/03/11 11:09:24]

Is it definitely the case that AllocationContextTracker will be initialized at
this point? If yes, you should probably add a comment explaining why.

[Primiano Tucci (use gerrit), 2016/03/11 13:57:57]

Good spot. It's definitely NOT the case (I mean, it can be non-initialized), for
instance, if the very first thing you intercept after installing the hooks is a
free (very likely can happen).
But the deal here is that we don't need the tracker in the RemoveAllocation()
function, so we don't care if that's initialized or not.
The reason why I bother initializing that in the malloc() is because I need it
few statements below when I do AllocationContextTracker::GetContextSnapshot()
and at that point it must have been initialized or a reentrancy will happen.

But yet again, this was another excellent spot, could have been a nasty bug,
thanks for the -pedantic mode.

[petrcermak, 2016/03/11 14:26:31]

Thanks for the detailed explanation. It might be worth adding a short comment
explaining this in here.

[Primiano Tucci (use gerrit), 2016/03/11 16:57:21]

Done

+  if (!allocation_register_)
+    return;
+  allocation_register_->Remove(address);
+}
+
 }  // namespace trace_event
 }  // namespace base