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..fc1ea046cc9e85d54188bab290413446b0bacc22
--- /dev/null
+++ b/gin/v8_isolate_memory_dump_provider.cc
@@ -0,0 +1,96 @@
+// 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/threading/platform_thread.h"
+#include "base/trace_event/process_memory_dump.h"
+#include "gin/public/isolate_holder.h"
+#include "v8/include/v8.h"
+
+namespace gin {
+
+namespace {
+

[Primiano Tucci (use gerrit), 2015/04/17 19:10:31]

don't need this extra \n

+const char kDumperFriendlyName[] = "V8IsolateHeap";
+const char kDumperRootName[] = "v8_root";

[Primiano Tucci (use gerrit), 2015/04/17 19:10:30]

kRootDumpName

[Primiano Tucci (use gerrit), 2015/04/17 19:10:31]

This should be just v8

+const char kDumperName[] = "v8_isolate";

[Primiano Tucci (use gerrit), 2015/04/17 19:10:31]

THis should be kIsolateDumpName

+const char kDumperOtherSpaces[] = "other_spaces";

[Primiano Tucci (use gerrit), 2015/04/17 19:10:31]

THis you can just put the string inline as you use it only once

+const char kAvailableSizeAttribute[] = "available_size_in_bytes";
+const char kPhysicalSizeAttribute[] = "physical_size_in_bytes";
+}  // namespace
+
+V8IsolateMemoryDumpProvider::V8IsolateMemoryDumpProvider(
+    IsolateHolder* isolate_holder) {

[Primiano Tucci (use gerrit), 2015/04/17 19:10:30]

After my change here you will need a call to the parent ctor as follows:
 : MemoryDumpProvider(base::MessageLoop::current()->task_runner()) 

+  DeclareAllocatorAttribute({kAvailableSizeAttribute, "bytes"});
+  DeclareAllocatorAttribute({kPhysicalSizeAttribute, "bytes"});
+  base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
+      this);
+  isolate_holder_ = isolate_holder;
+}
+
+V8IsolateMemoryDumpProvider::~V8IsolateMemoryDumpProvider() {
+  base::trace_event::MemoryDumpManager::GetInstance()->UnregisterDumpProvider(
+      this);
+}
+
+// Called at trace dump point time. Creates a snapshot the memory counters for

[Primiano Tucci (use gerrit), 2015/04/17 19:10:30]

snapshot +with the...

+// the current process.

[Primiano Tucci (use gerrit), 2015/04/17 19:10:31]

this is actually the current isolate,not process

+bool V8IsolateMemoryDumpProvider::DumpInto(
+    base::trace_event::ProcessMemoryDump* pmd) {
+  v8::HeapStatistics heap_statistics;
+  isolate_holder_->isolate()->GetHeapStatistics(&heap_statistics);
+
+  base::trace_event::MemoryAllocatorDump* root =
+      pmd->GetAllocatorDump(kDumperRootName);
+  if (root == nullptr) {
+    root = pmd->CreateAllocatorDump(kDumperRootName);
+  }
+
+  int thread_id = base::PlatformThread::CurrentId();
+  std::stringstream dumper_name;

[Primiano Tucci (use gerrit), 2015/04/17 19:10:30]

even if it works anyways, here you need an #include for whoever defines
stringstream

+  dumper_name << kDumperName << thread_id;

[Primiano Tucci (use gerrit), 2015/04/17 19:10:30]

probably add an underscore as separator here.
ALso please add a TODO, the thread_id is soon not going to be unique enough if
we get more isolates per thread.

+  base::trace_event::MemoryAllocatorDump* isolate_dump =
+      pmd->CreateAllocatorDump(dumper_name.str(), root);
+
+  size_t known_spaces_used_size = 0;
+  size_t known_spaces_size = 0;
+  int number_of_spaces = isolate_holder_->isolate()->NumberOfHeapSpaces();
+  for (int space = 0; space < number_of_spaces; space++) {

[Primiano Tucci (use gerrit), 2015/04/17 19:10:31]

probably this should be size_t (match the API)

+    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;
+
+    base::trace_event::MemoryAllocatorDump* space_dump =
+        pmd->CreateAllocatorDump(space_statistics.space_name(), isolate_dump);

[Primiano Tucci (use gerrit), 2015/04/17 19:10:30]

Note to myself (primiano): probably this will need to be changed when I'll
change the CreateAllocatorDump API.

+    space_dump->set_physical_size_in_bytes(space_size);
+    space_dump->set_allocated_objects_count(0);
+    space_dump->set_allocated_objects_size_in_bytes(space_used_size);
+    space_dump->SetExtraAttribute(kAvailableSizeAttribute,
+                                  space_statistics.space_available_size());
+    space_dump->SetExtraAttribute(kPhysicalSizeAttribute,

[Primiano Tucci (use gerrit), 2015/04/17 19:10:31]

Maybe add a TODO here, probably this should be our physical_size_in_bytes and
the other the attrib. Let's discuss this with a V8 owner.

+                                  space_statistics.physical_space_size());
+  }
+
+  base::trace_event::MemoryAllocatorDump* space_dump =

[Primiano Tucci (use gerrit), 2015/04/17 19:10:31]

Add a comment explaining that here you do the compute the final diff.

+      pmd->CreateAllocatorDump(kDumperOtherSpaces, isolate_dump);
+  space_dump->set_physical_size_in_bytes(heap_statistics.total_heap_size() -
+                                         known_spaces_size);
+  space_dump->set_allocated_objects_count(0);
+  space_dump->set_allocated_objects_size_in_bytes(
+      heap_statistics.used_heap_size() - known_spaces_used_size);
+
+  return true;
+}
+
+const char* V8IsolateMemoryDumpProvider::GetFriendlyName() const {
+  return kDumperFriendlyName;
+}
+
+}  // namespace gin