From 429ce41f4b86b7c6a9bfe61daae10d75c875735e Mon Sep 17 00:00:00 2001
From: "alph@chromium.org"
 <alph@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Mon, 17 Feb 2014 15:24:39 +0000
Subject: [PATCH] Make a single HeapEntry per single JSArrayBuffer data in heap
 snapshot.

It turned out that JSArrayBuffer's may share their backing_store so
the backing_store should go through hash map registration just like
other heap objects, so they won't be reported twice.

BUG=341741
LOG=N
R=dslomov@chromium.org, yurys@chromium.org

Review URL: https://codereview.chromium.org/166993002

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19416 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/heap-snapshot-generator.cc    | 24 ++++++++++++++---
 src/heap-snapshot-generator.h     |  8 +++---
 test/cctest/test-heap-profiler.cc | 45 +++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 8 deletions(-)

diff --git a/src/heap-snapshot-generator.cc b/src/heap-snapshot-generator.cc
index 8a9b0d6b3..ec6e10bef 100644
--- a/src/heap-snapshot-generator.cc
+++ b/src/heap-snapshot-generator.cc
@@ -1456,6 +1456,23 @@ void V8HeapExplorer::ExtractAllocationSiteReferences(int entry,
 }
 
 
+class JSArrayBufferDataEntryAllocator : public HeapEntriesAllocator {
+ public:
+  JSArrayBufferDataEntryAllocator(int size, V8HeapExplorer* explorer)
+      : size_(size)
+      , explorer_(explorer) {
+  }
+  virtual HeapEntry* AllocateEntry(HeapThing ptr) {
+    return explorer_->AddEntry(
+        static_cast<Address>(ptr),
+        HeapEntry::kNative, "system / JSArrayBufferData", size_);
+  }
+ private:
+  int size_;
+  V8HeapExplorer* explorer_;
+};
+
+
 void V8HeapExplorer::ExtractJSArrayBufferReferences(
     int entry, JSArrayBuffer* buffer) {
   SetWeakReference(buffer, entry, "weak_next", buffer->weak_next(),
@@ -1468,10 +1485,9 @@ void V8HeapExplorer::ExtractJSArrayBufferReferences(
     return;
   size_t data_size = NumberToSize(heap_->isolate(), buffer->byte_length());
   CHECK(data_size <= static_cast<size_t>(kMaxInt));
-  HeapEntry* data_entry = AddEntry(
-      static_cast<Address>(buffer->backing_store()),
-      HeapEntry::kNative, "system / ArrayBufferData",
-      static_cast<int>(data_size));
+  JSArrayBufferDataEntryAllocator allocator(static_cast<int>(data_size), this);
+  HeapEntry* data_entry =
+      filler_->FindOrAddEntry(buffer->backing_store(), &allocator);
   filler_->SetNamedReference(HeapGraphEdge::kInternal,
                              entry, "backing_store", data_entry);
 }
diff --git a/src/heap-snapshot-generator.h b/src/heap-snapshot-generator.h
index 9ea366e6c..35e80be72 100644
--- a/src/heap-snapshot-generator.h
+++ b/src/heap-snapshot-generator.h
@@ -386,6 +386,10 @@ class V8HeapExplorer : public HeapEntriesAllocator {
   void TagGlobalObjects();
   void TagCodeObject(Code* code);
   void TagBuiltinCodeObject(Code* code, const char* name);
+  HeapEntry* AddEntry(Address address,
+                      HeapEntry::Type type,
+                      const char* name,
+                      int size);
 
   static String* GetConstructorName(JSObject* object);
 
@@ -396,10 +400,6 @@ class V8HeapExplorer : public HeapEntriesAllocator {
   HeapEntry* AddEntry(HeapObject* object,
                       HeapEntry::Type type,
                       const char* name);
-  HeapEntry* AddEntry(Address address,
-                      HeapEntry::Type type,
-                      const char* name,
-                      int size);
 
   const char* GetSystemEntryName(HeapObject* object);
 
diff --git a/test/cctest/test-heap-profiler.cc b/test/cctest/test-heap-profiler.cc
index 436f2559b..5c102e0c5 100644
--- a/test/cctest/test-heap-profiler.cc
+++ b/test/cctest/test-heap-profiler.cc
@@ -2388,6 +2388,51 @@ TEST(ArrayBufferAndArrayBufferView) {
 }
 
 
+static int GetRetainersCount(const v8::HeapSnapshot* snapshot,
+                             const v8::HeapGraphNode* node) {
+  int count = 0;
+  for (int i = 0, l = snapshot->GetNodesCount(); i < l; ++i) {
+    const v8::HeapGraphNode* parent = snapshot->GetNode(i);
+    for (int j = 0, l2 = parent->GetChildrenCount(); j < l2; ++j) {
+      if (parent->GetChild(j)->GetToNode() == node) {
+        ++count;
+      }
+    }
+  }
+  return count;
+}
+
+
+TEST(ArrayBufferSharedBackingStore) {
+  LocalContext env;
+  v8::HandleScope scope(env->GetIsolate());
+  v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
+  CompileRun("sin1 = Math.sin(1);");
+  LocalContext env2;
+  CompileRun("sin2 = Math.sin(2);");
+  const v8::HeapSnapshot* snapshot =
+      heap_profiler->TakeHeapSnapshot(v8_str("snapshot"));
+  CHECK(ValidateSnapshot(snapshot));
+  // The 0th-child is (GC Roots), 1st is the user root.
+  const v8::HeapGraphNode* global =
+      snapshot->GetRoot()->GetChild(1)->GetToNode();
+  const v8::HeapGraphNode* builtins =
+      GetProperty(global, v8::HeapGraphEdge::kInternal, "builtins");
+  CHECK_NE(NULL, builtins);
+  const v8::HeapGraphNode* sin_table =
+      GetProperty(builtins, v8::HeapGraphEdge::kProperty, "kSinTable");
+  CHECK_NE(NULL, sin_table);
+  const v8::HeapGraphNode* buffer =
+      GetProperty(sin_table, v8::HeapGraphEdge::kInternal, "buffer");
+  CHECK_NE(NULL, buffer);
+  const v8::HeapGraphNode* backing_store =
+      GetProperty(buffer, v8::HeapGraphEdge::kInternal, "backing_store");
+  CHECK_NE(NULL, backing_store);
+  int retainers = GetRetainersCount(snapshot, backing_store);
+  CHECK_EQ(2, retainers);
+}
+
+
 TEST(BoxObject) {
   v8::Isolate* isolate = CcTest::isolate();
   v8::HandleScope scope(isolate);
-- 
2.34.1