Adding v8_isolate_memory_dump_provider.

gin/v8_isolate_memory_dump_provider.cc

Index: gin/v8_isolate_memory_dump_provider.cc
diff --git a/gin/v8_isolate_memory_dump_provider.cc b/gin/v8_isolate_memory_dump_provider.cc
new file mode 100644
index 0000000000000000000000000000000000000000..0c4552c11917a2f4f7ba8fc6d65e97f5a25251ce
--- /dev/null
+++ b/gin/v8_isolate_memory_dump_provider.cc
@@ -0,0 +1,89 @@
+// 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 "gin/v8_isolate_memory_dump_provider.h"
+
+#include "base/message_loop/message_loop.h"
+#include "base/strings/stringprintf.h"
+#include "base/trace_event/memory_dump_manager.h"
+#include "base/trace_event/process_memory_dump.h"
+#include "gin/public/isolate_holder.h"
+#include "v8/include/v8.h"
+
+namespace gin {
+
+namespace {
+const char kDumperFriendlyName[] = "V8IsolateHeap";
+const char kRootDumpName[] = "v8";
+const char kIsolateDumpName[] = "isolate_%p/%s";
+const char kAvailableSizeAttribute[] = "available_size_in_bytes";
+}  // namespace
+
+V8IsolateMemoryDumpProvider::V8IsolateMemoryDumpProvider(
+    IsolateHolder* isolate_holder)
+    : MemoryDumpProvider(base::MessageLoop::current()->task_runner()) {

[rmcilroy, 2015/04/21 21:57:26]

I think you should be using base::ThreadTaskRunnerHandle::Get() instead of
base::MessageLoop::current()->task_runner() (Sami has been trying to move things
to this API to ensure that the correct scheduler driven task runner is used
wherever possible rather than posting directly to the message loop).

[ssid, 2015/04/22 10:41:35]

Thanks, changed.

+  DeclareAllocatorAttribute(kRootDumpName, kAvailableSizeAttribute, "bytes");
+  base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
+      this);
+  isolate_holder_ = isolate_holder;

[rmcilroy, 2015/04/21 21:57:25]

nit - move this to the initializer list.

[ssid, 2015/04/22 10:41:35]

Done.

+}
+
+V8IsolateMemoryDumpProvider::~V8IsolateMemoryDumpProvider() {
+  base::trace_event::MemoryDumpManager::GetInstance()->UnregisterDumpProvider(
+      this);
+}
+
+// Called at trace dump point time. Creates a snapshot with the memory counters
+// for the current isolate.
+bool V8IsolateMemoryDumpProvider::DumpInto(
+    base::trace_event::ProcessMemoryDump* pmd) {
+  v8::HeapStatistics heap_statistics;
+  isolate_holder_->isolate()->GetHeapStatistics(&heap_statistics);
+
+  size_t known_spaces_used_size = 0;
+  size_t known_spaces_size = 0;
+  size_t number_of_spaces = isolate_holder_->isolate()->NumberOfHeapSpaces();
+  for (size_t space = 0; space < number_of_spaces; space++) {
+    v8::HeapSpaceStatistics space_statistics;
+    isolate_holder_->isolate()->GetHeapSpaceStatistics(&space_statistics,
+                                                       space);
+    size_t space_size = space_statistics.space_size();
+    size_t space_used_size = space_statistics.space_used_size();
+
+    known_spaces_size += space_size;
+    known_spaces_used_size += space_used_size;
+
+    std::string allocator_name =
+        base::StringPrintf(kIsolateDumpName, isolate_holder_->isolate(),

[rmcilroy, 2015/04/21 21:57:25]

nit - I would prefer you just had the string literal here rather than using a
constant so that it is obvious what is being formatted.

[ssid, 2015/04/22 10:41:35]

This is kept as const at the top because each dump provider we have the names at
the start of the file. (The root and the main child dumpers). This is general
style I followed. Also it is used at 2 places.

+                           space_statistics.space_name());
+    base::trace_event::MemoryAllocatorDump* space_dump =
+        pmd->CreateAllocatorDump(kRootDumpName, allocator_name.c_str());
+    space_dump->set_physical_size_in_bytes(static_cast<int>((space_size)));
+    space_dump->set_allocated_objects_count(0);

[rmcilroy, 2015/04/21 21:57:26]

Add a todo to add support for objects_count? It should be possible in V8 to at
least get a count of the number of live objects at the last GC if it is
something you would want to track?

[ssid, 2015/04/22 10:41:35]

Done.

+    space_dump->set_allocated_objects_size_in_bytes(
+        static_cast<int>(space_used_size));
+    space_dump->SetAttribute(
+        kAvailableSizeAttribute,
+        static_cast<int>(space_statistics.space_available_size()));
+  }
+
+  // Compute the rest of the memory, not accounted by the spaces above.
+  std::string allocator_name = base::StringPrintf(
+      kIsolateDumpName, isolate_holder_->isolate(), "other_spaces");
+  base::trace_event::MemoryAllocatorDump* other_spaces_dump =
+      pmd->CreateAllocatorDump(kRootDumpName, allocator_name.c_str());
+  other_spaces_dump->set_physical_size_in_bytes(
+      heap_statistics.total_heap_size() - known_spaces_size);
+  other_spaces_dump->set_allocated_objects_count(0);
+  other_spaces_dump->set_allocated_objects_size_in_bytes(
+      heap_statistics.used_heap_size() - known_spaces_used_size);

[rmcilroy, 2015/04/21 21:57:25]

Should you also add kAvailableSizeAttribute as zero currently with a todo to add
support to the V8 API for Available in HeapStatistics? (it would be easy to add
since there is already an Heap::Available() method in V8.

[ssid, 2015/04/22 10:41:35]

Done.

+

[rmcilroy, 2015/04/21 21:57:26]

Primiano: I don't know how the hierarchy of allocators works for these dumpers,
but as I remember the original plan had a top-level node with the total heap
use/committed, then branched down to per-space sizes - is this something which
is calculated offline now or should this dumper be explicitly adding the
top-level node for heap_statistics.total_heap_size() and 
heap_statistics.used_heap_size()

+  return true;
+}
+
+const char* V8IsolateMemoryDumpProvider::GetFriendlyName() const {
+  return kDumperFriendlyName;
+}
+
+}  // namespace gin