Adding heap-page statistics to blink-gc memory dump provider.

Source/platform/heap/Heap.cpp

Index: Source/platform/heap/Heap.cpp
diff --git a/Source/platform/heap/Heap.cpp b/Source/platform/heap/Heap.cpp
index 9d49bed23645c37e19ce7b5ce5f9afb478d95035..9ca71a0338fe103b443067b92cecc000478d802e 100644
--- a/Source/platform/heap/Heap.cpp
+++ b/Source/platform/heap/Heap.cpp
@@ -217,11 +217,12 @@ void BaseHeap::cleanupPages()
 
 void BaseHeap::takeSnapshot(const String& dumpBaseName)
 {
+    WebMemoryAllocatorDump* allocatorDump = BlinkGCMemoryDumpProvider::instance()->createMemoryAllocatorDumpForCurrentGC(dumpBaseName);

[haraken, 2015/05/29 02:11:30]

I'm just curious but does the place we create the allocatorDump matter? i.e., is
it important that we create the allocatorDump for the BaseHeap before creating
allocatorDumps for BaseHeapPages?

[Primiano Tucci (use gerrit), 2015/05/29 08:32:50]

Do you mean the order or the call site?
The order in which the dumps are created is irrelevant (in fact, at the end they
end up in a dictionary which will hash them).

The callsite: well, in general we have to make sure that we create all our dumps
before returning the outer onMemoryDump(WebProcessMemoryDump*) call.

Here in BlinkGC we use an extra little trick: we create an extra
WebProcessMemoryDump (see [1]) that is always available and we use as a "buffer"
to create all the allocatorDumps like this one here when traversing the oilpan
GC structs. At the end we simply merge (see [2])
the buffer WPMD into the real one that meminfra expects (the argument of
onMemoryDump).
In this way we can guarantee that even if, for whatever reason, you force a
Heap::collectGarbage(takeSnapshow) outside of onMemoryDump(...), everything
works fine (you will just end up collecting useless WPMD that will be dropped
before the next dump).

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[2]
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[ssid, 2015/05/29 10:57:19]

Just had a chat with primiano@. It is not necessary to move this, but looks
cleaner to create the root after children.

     size_t pageCount = 0;
     for (BasePage* page = m_firstUnsweptPage; page; page = page->next()) {
+        page->takeSnapshot(dumpBaseName, pageCount);

[haraken, 2015/05/29 02:11:30]

pageCount => pageIndex

[ssid, 2015/05/29 10:57:19]

Done.

[ssid, 2015/05/29 10:57:19]

Done.

         pageCount++;
     }
-    WebMemoryAllocatorDump* allocatorDump = BlinkGCMemoryDumpProvider::instance()->createMemoryAllocatorDumpForCurrentGC(dumpBaseName);
     allocatorDump->AddScalar("blink_page_count", "objects", pageCount);
 }
 
@@ -1394,6 +1395,44 @@ void NormalPage::markOrphaned()
     BasePage::markOrphaned();
 }
 
+void NormalPage::takeSnapshot(const String& dumpBaseName, size_t pageIndex)
+{
+    String pageName = dumpBaseName.isolatedCopy();

[haraken, 2015/05/29 02:11:30]

Instead of passing const String& and calling isolatedCopy, I'd prefer just
passing 'String'.

[Primiano Tucci (use gerrit), 2015/05/29 08:32:50]

+1

+    pageName.append(String::format("/page_%zu", pageIndex));
+    WebMemoryAllocatorDump* pageDump = BlinkGCMemoryDumpProvider::instance()->createMemoryAllocatorDumpForCurrentGC(pageName);
+
+    HeapObjectHeader* header = nullptr;
+    size_t objectCount = 0;
+    size_t liveCount = 0;
+    size_t deadCount = 0;
+    size_t freeCount = 0;
+    size_t liveSize = 0;
+    size_t deadSize = 0;
+    size_t freeSize = 0;
+    for (Address addr = payload(); addr < payloadEnd(); addr += header->size()) {

[haraken, 2015/05/29 02:11:30]

addr => headerAddress

[ssid, 2015/05/29 10:57:19]

Done.

+        header = reinterpret_cast<HeapObjectHeader*>(addr);
+        objectCount++;
+        if (header->isFree()) {
+            freeCount++;
+            freeSize += header->size();
+        } else if (header->isMarked()) {
+            liveCount++;
+            liveSize += header->size();
+        } else {
+            deadCount++;
+            deadSize += header->size();
+        }
+    }
+
+    pageDump->AddScalar("object_count", "objects", objectCount);

[haraken, 2015/05/29 02:11:30]

Remove objectCount. The freed area is not an object, so it doesn't make sense to
count up the total number of headers and report it.

[ssid, 2015/05/29 10:57:19]

Done, thanks.

+    pageDump->AddScalar("live_count", "objects", liveCount);
+    pageDump->AddScalar("dead_count", "objects", deadCount);
+    pageDump->AddScalar("free_count", "objects", freeCount);
+    pageDump->AddScalar("live_size", "bytes", liveSize);
+    pageDump->AddScalar("dead_size", "bytes", deadSize);
+    pageDump->AddScalar("free_size", "bytes", freeSize);
+}
+
 #if ENABLE(GC_PROFILING)
 const GCInfo* NormalPage::findGCInfo(Address address)
 {
@@ -1570,6 +1609,32 @@ void LargeObjectPage::markOrphaned()
     BasePage::markOrphaned();
 }
 
+void LargeObjectPage::takeSnapshot(const String& dumpBaseName, size_t pageIndex)
+{
+    String pageName = dumpBaseName.isolatedCopy();
+    pageName.append(String::format("/page_%zu", pageIndex));
+    WebMemoryAllocatorDump* pageDump = BlinkGCMemoryDumpProvider::instance()->createMemoryAllocatorDumpForCurrentGC(pageName);
+
+    size_t liveSize = 0;
+    size_t deadSize = 0;
+    size_t liveCount = 0;
+    size_t deadCount = 0;
+    HeapObjectHeader* header = heapObjectHeader();
+    if (header->isMarked()) {
+        liveCount = 1;
+        liveSize += header->size();
+    } else {
+        deadCount = 1;
+        deadSize += header->size();
+    }
+
+    pageDump->AddScalar("object_count", "objects", 1u);

[haraken, 2015/05/29 02:11:30]

Remove this.

[ssid, 2015/05/29 10:57:19]

Done.

+    pageDump->AddScalar("live_count", "objects", liveCount);
+    pageDump->AddScalar("dead_count", "objects", deadCount);
+    pageDump->AddScalar("live_size", "bytes", liveSize);
+    pageDump->AddScalar("dead_size", "bytes", deadSize);
+}
+
 #if ENABLE(GC_PROFILING)
 const GCInfo* LargeObjectPage::findGCInfo(Address address)
 {