Heap Profiling for Oilpan

third_party/WebKit/Source/platform/heap/BlinkGCMemoryDumpProvider.cpp

Index: third_party/WebKit/Source/platform/heap/BlinkGCMemoryDumpProvider.cpp
diff --git a/third_party/WebKit/Source/platform/heap/BlinkGCMemoryDumpProvider.cpp b/third_party/WebKit/Source/platform/heap/BlinkGCMemoryDumpProvider.cpp
index 6f4f4e723f48ab675ab24bb7c2c11af92c8b974a..ac008287ba2dca9361ca24df553e11e25ff56c39 100644
--- a/third_party/WebKit/Source/platform/heap/BlinkGCMemoryDumpProvider.cpp
+++ b/third_party/WebKit/Source/platform/heap/BlinkGCMemoryDumpProvider.cpp
@@ -4,6 +4,9 @@
 
 #include "platform/heap/BlinkGCMemoryDumpProvider.h"
 
+#include "base/trace_event/heap_profiler_allocation_context_tracker.h"
+#include "base/trace_event/heap_profiler_allocation_register.h"
+#include "base/trace_event/trace_event_memory_overhead.h"
 #include "platform/heap/Handle.h"
 #include "public/platform/Platform.h"
 #include "public/platform/WebMemoryAllocatorDump.h"
@@ -28,6 +31,16 @@ void dumpMemoryTotals(blink::WebProcessMemoryDump* memoryDump)
     objectsDump->addScalar("size", "bytes", Heap::allocatedObjectSize() + Heap::markedObjectSize());
 }
 
+void reportAllocation(Address address, size_t size)
+{
+    BlinkGCMemoryDumpProvider::instance()->insert(address, size);
+}
+
+void reportFree(Address address)
+{
+    BlinkGCMemoryDumpProvider::instance()->remove(address);
+}
+
 } // namespace
 
 BlinkGCMemoryDumpProvider* BlinkGCMemoryDumpProvider::instance()
@@ -50,11 +63,42 @@ bool BlinkGCMemoryDumpProvider::onMemoryDump(WebMemoryDumpLevelOfDetail levelOfD
     Heap::collectGarbage(BlinkGC::NoHeapPointersOnStack, BlinkGC::TakeSnapshot, BlinkGC::ForcedGC);
     dumpMemoryTotals(memoryDump);
 
+    // TODO(hajimehoshi): This doesn't work causing assert error: [process_memory_dump.cc(149)] Check failed: 0ul == allocator_dumps_.count(mad->absolute_name()) (0 vs. 1)

[hajimehoshi, 2016/02/16 11:02:07]

primiano: I simply copied this dump heap usage logic from 
PartitionAllocMemoryDumpProvider::onMemoryDump but this doesn't work with
assertion error as the comment says. It looks like onMemoryDump in
BlinkGCMemoryDumpProvider is much different from that in
PartitionAllocMemoryDumpProvider and BlinkGCMemoryDumpProvider. Would you tell
me what I should fix?

[Primiano Tucci (use gerrit), 2016/02/16 11:16:42]

Oh I think I know what it is: is the overhead dumping in
web_process_memory_dump_impl.cc (WebProcessMemoryDumpImpl::dumpHeapUsage())
which always dumps the overhead estimation with 
overhead.DumpInto("tracing/heap_profiler")

Before this CL you have only one tracing/heap_profiler (for partition alloc),
now you have two.

Simply change the code in WebProcessMemoryDumpImpl::dumpHeapUsage and make it
strcat the |allocator_name| when you do the overhead dumping, so it becomes
something like "tracing/blink_gc_heap_profiler" and
"tracing/partition_alloc_heap_profiler".
SHould be a one line fix

[hajimehoshi, 2016/02/17 09:19:40]

Thanks, done.

+    if (m_isHeapProfilingEnabled) {
+        base::trace_event::TraceEventMemoryOverhead overhead;
+        base::hash_map<base::trace_event::AllocationContext, size_t> bytesByContext;
+        {
+            MutexLocker locker(m_allocationRegisterMutex);
+            for (const auto& allocSize : *m_allocationRegister)
+                bytesByContext[allocSize.context] += allocSize.size;
+
+            m_allocationRegister->EstimateTraceMemoryOverhead(&overhead);
+        }
+        memoryDump->dumpHeapUsage(bytesByContext, overhead, "blink_gc");
+    }
+
     // Merge all dumps collected by Heap::collectGarbage.
     memoryDump->takeAllDumpsFrom(m_currentProcessMemoryDump.get());
     return true;
 }
 
+void BlinkGCMemoryDumpProvider::onHeapProfilingEnabled(bool enabled)
+{
+    if (enabled) {
+        {
+            MutexLocker locker(m_allocationRegisterMutex);
+            if (!m_allocationRegister)
+                m_allocationRegister = adoptPtr(new base::trace_event::AllocationRegister());
+        }
+        HeapAllocHooks::setAllocationHook(reportAllocation);
+        HeapAllocHooks::setFreeHook(reportFree);
+    } else {
+        HeapAllocHooks::setAllocationHook(nullptr);
+        HeapAllocHooks::setFreeHook(nullptr);
+    }

[Primiano Tucci (use gerrit), 2016/02/16 11:16:42]

similarly to the other CL, I think you might want to tear down the allocation
register here?

[hajimehoshi, 2016/02/17 09:19:40]

In PartitionAllocMemoryDumpProvider, no tearing down happens. Which CL did you
mean?

[haraken, 2016/02/17 09:23:05]

Would it make sense to support disabling the heap profiling? Given that
'--enable-heap-profiling' is the only way to enable the heap profiling (and it
doesn't make sense to disable it later), it would make more sense to change the
signature of this method to onHeapProfilingEnabled() (without taking the
|enabled| parameter)?

[hajimehoshi, 2016/02/17 09:57:38]

+1 to haraken

[Primiano Tucci (use gerrit), 2016/02/17 10:13:46]

You are right. The only motivation I see for tear-down is that after you finish
the tracing session you go back to a normal performance/memory state, freeing
out hooks and memory used for metadata.

+    m_isHeapProfilingEnabled = enabled;
+}
+
 WebMemoryAllocatorDump* BlinkGCMemoryDumpProvider::createMemoryAllocatorDumpForCurrentGC(const String& absoluteName)
 {
     return m_currentProcessMemoryDump->createMemoryAllocatorDump(absoluteName);
@@ -67,7 +111,25 @@ void BlinkGCMemoryDumpProvider::clearProcessDumpForCurrentGC()
 
 BlinkGCMemoryDumpProvider::BlinkGCMemoryDumpProvider()
     : m_currentProcessMemoryDump(adoptPtr(Platform::current()->createProcessMemoryDump()))
+    , m_isHeapProfilingEnabled(false)
+{
+}
+
+void BlinkGCMemoryDumpProvider::insert(Address address, size_t size)
+{
+    base::trace_event::AllocationContext context = base::trace_event::AllocationContextTracker::GetContextSnapshot();
+    // TODO(hajimehoshi): Implement to use a correct type name.
+    context.type_name = "(other)";

[Primiano Tucci (use gerrit), 2016/02/16 11:16:42]

I think you can leave it empty, and will just appear as unknown

[hajimehoshi, 2016/02/17 09:19:40]

Done.

+    MutexLocker locker(m_allocationRegisterMutex);
+    if (m_allocationRegister)
+        m_allocationRegister->Insert(address, size, context);
+}
+
+void BlinkGCMemoryDumpProvider::remove(Address address)
 {
+    MutexLocker locker(m_allocationRegisterMutex);
+    if (m_allocationRegister)
+        m_allocationRegister->Remove(address);
 }
 
 } // namespace blink