Fix huge heap snapshot when a heavily shared context has many variables

Prevously v8 put a link to each context variable into a function where
the variable is visible.
Because of that if there are N functions sharing a context having M variables
then N*M links were created for the snapshot.
The fix makes v8 to put the links into the context object.

BUG=145687
TEST=test-heap-snapshot/ManyLocalsInSharedContext

Review URL: https://codereview.appspot.com/7715044

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@13936 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/include/v8-profiler.h b/include/v8-profiler.h
index 4d3597a..c9eab3f 100644 (file)
--- a/include/v8-profiler.h
+++ b/include/v8-profiler.h
@@ -258,8 +258,9 @@ class V8EXPORT HeapGraphNode {
     kRegExp = 6,      // RegExp.
     kHeapNumber = 7,  // Number stored in the heap.
     kNative = 8,      // Native object (not from V8 heap).
-    kSynthetic = 9    // Synthetic object, usualy used for grouping
+    kSynthetic = 9,   // Synthetic object, usualy used for grouping
                       // snapshot items together.
+    kContext = 10     // Context
   };
 
   /** Returns node type (see HeapGraphNode::Type). */

diff --git a/src/heap-snapshot-generator.cc b/src/heap-snapshot-generator.cc
index 5f46678..cdd5a07 100644 (file)
--- a/src/heap-snapshot-generator.cc
+++ b/src/heap-snapshot-generator.cc
@@ -174,6 +174,7 @@ const char* HeapEntry::TypeAsString() {
     case kHeapNumber: return "/number/";
     case kNative: return "/native/";
     case kSynthetic: return "/synthetic/";
+    case kContext: return "/context/";
     default: return "???";
   }
 }
@@ -826,7 +827,7 @@ HeapEntry* V8HeapExplorer::AddEntry(HeapObject* object) {
   } else if (object->IsNativeContext()) {
     return AddEntry(object, HeapEntry::kHidden, "system / NativeContext");
   } else if (object->IsContext()) {
-    return AddEntry(object, HeapEntry::kHidden, "system / Context");
+    return AddEntry(object, HeapEntry::kContext, "system / Context");
   } else if (object->IsFixedArray() ||
              object->IsFixedDoubleArray() ||
              object->IsByteArray() ||
@@ -1097,6 +1098,27 @@ void V8HeapExplorer::ExtractStringReferences(int entry, String* string) {
 
 
 void V8HeapExplorer::ExtractContextReferences(int entry, Context* context) {
+  if (context == context->declaration_context()) {
+    ScopeInfo* scope_info = context->closure()->shared()->scope_info();
+    // Add context allocated locals.
+    int context_locals = scope_info->ContextLocalCount();
+    for (int i = 0; i < context_locals; ++i) {
+      String* local_name = scope_info->ContextLocalName(i);
+      int idx = Context::MIN_CONTEXT_SLOTS + i;
+      SetContextReference(context, entry, local_name, context->get(idx),
+                          Context::OffsetOfElementAt(idx));
+    }
+    if (scope_info->HasFunctionName()) {
+      String* name = scope_info->FunctionName();
+      VariableMode mode;
+      int idx = scope_info->FunctionContextSlotIndex(name, &mode);
+      if (idx >= 0) {
+        SetContextReference(context, entry, name, context->get(idx),
+                            Context::OffsetOfElementAt(idx));
+      }
+    }
+  }
+
 #define EXTRACT_CONTEXT_FIELD(index, type, name) \
   SetInternalReference(context, entry, #name, context->get(Context::index), \
       FixedArray::OffsetOfElementAt(Context::index));
@@ -1286,26 +1308,6 @@ void V8HeapExplorer::ExtractClosureReferences(JSObject* js_obj, int entry) {
       SetNativeBindReference(js_obj, entry, reference_name,
                              bindings->get(i));
     }
-  } else {
-    Context* context = func->context()->declaration_context();
-    ScopeInfo* scope_info = context->closure()->shared()->scope_info();
-    // Add context allocated locals.
-    int context_locals = scope_info->ContextLocalCount();
-    for (int i = 0; i < context_locals; ++i) {
-      String* local_name = scope_info->ContextLocalName(i);
-      int idx = Context::MIN_CONTEXT_SLOTS + i;
-      SetClosureReference(js_obj, entry, local_name, context->get(idx));
-    }
-
-    // Add function variable.
-    if (scope_info->HasFunctionName()) {
-      String* name = scope_info->FunctionName();
-      VariableMode mode;
-      int idx = scope_info->FunctionContextSlotIndex(name, &mode);
-      if (idx >= 0) {
-        SetClosureReference(js_obj, entry, name, context->get(idx));
-      }
-    }
   }
 }
 
@@ -1580,16 +1582,18 @@ bool V8HeapExplorer::IsEssentialObject(Object* object) {
 }
 
 
-void V8HeapExplorer::SetClosureReference(HeapObject* parent_obj,
+void V8HeapExplorer::SetContextReference(HeapObject* parent_obj,
                                          int parent_entry,
                                          String* reference_name,
-                                         Object* child_obj) {
+                                         Object* child_obj,
+                                         int field_offset) {
   HeapEntry* child_entry = GetEntry(child_obj);
   if (child_entry != NULL) {
     filler_->SetNamedReference(HeapGraphEdge::kContextVariable,
                                parent_entry,
                                collection_->names()->GetName(reference_name),
                                child_entry);
+    IndexedReferencesExtractor::MarkVisitedField(parent_obj, field_offset);
   }
 }
 
@@ -2576,7 +2580,8 @@ void HeapSnapshotJSONSerializer::SerializeSnapshot() {
             JSON_S("regexp") ","
             JSON_S("number") ","
             JSON_S("native") ","
-            JSON_S("synthetic")) ","
+            JSON_S("synthetic") ","
+            JSON_S("context")) ","
         JSON_S("string") ","
         JSON_S("number") ","
         JSON_S("number") ","

diff --git a/src/heap-snapshot-generator.h b/src/heap-snapshot-generator.h
index fe15715..c952381 100644 (file)
--- a/src/heap-snapshot-generator.h
+++ b/src/heap-snapshot-generator.h
@@ -98,7 +98,8 @@ class HeapEntry BASE_EMBEDDED {
     kRegExp = v8::HeapGraphNode::kRegExp,
     kHeapNumber = v8::HeapGraphNode::kHeapNumber,
     kNative = v8::HeapGraphNode::kNative,
-    kSynthetic = v8::HeapGraphNode::kSynthetic
+    kSynthetic = v8::HeapGraphNode::kSynthetic,
+    kContext = v8::HeapGraphNode::kContext
   };
   static const int kNoEntry;
 
@@ -479,10 +480,11 @@ class V8HeapExplorer : public HeapEntriesAllocator {
   void ExtractElementReferences(JSObject* js_obj, int entry);
   void ExtractInternalReferences(JSObject* js_obj, int entry);
   bool IsEssentialObject(Object* object);
-  void SetClosureReference(HeapObject* parent_obj,
+  void SetContextReference(HeapObject* parent_obj,
                            int parent,
                            String* reference_name,
-                           Object* child);
+                           Object* child,
+                           int field_offset);
   void SetNativeBindReference(HeapObject* parent_obj,
                               int parent,
                               const char* reference_name,

diff --git a/test/cctest/test-heap-profiler.cc b/test/cctest/test-heap-profiler.cc
index 8ef970f..e1dea9d 100644 (file)
--- a/test/cctest/test-heap-profiler.cc
+++ b/test/cctest/test-heap-profiler.cc
@@ -1726,3 +1726,46 @@ TEST(MapHasDescriptorsAndTransitions) {
       map, v8::HeapGraphEdge::kInternal, "transitions");
   CHECK_EQ(NULL, own_transitions);
 }
+
+
+TEST(ManyLocalsInSharedContext) {
+  v8::HandleScope scope;
+  LocalContext env;
+  int num_objects = 5000;
+  CompileRun(
+      "var n = 5000;"
+      "var result = [];"
+      "result.push('(function outer() {');"
+      "for (var i = 0; i < n; i++) {"
+      "    var f = 'function f_' + i + '() { ';"
+      "    if (i > 0)"
+      "        f += 'f_' + (i - 1) + '();';"
+      "    f += ' }';"
+      "    result.push(f);"
+      "}"
+      "result.push('return f_' + (n - 1) + ';');"
+      "result.push('})()');"
+      "var ok = eval(result.join('\\n'));");
+  const v8::HeapSnapshot* snapshot =
+      v8::HeapProfiler::TakeSnapshot(v8_str("snapshot"));
+  const v8::HeapGraphNode* global = GetGlobalObject(snapshot);
+  CHECK_NE(NULL, global);
+  const v8::HeapGraphNode* ok_object =
+      GetProperty(global, v8::HeapGraphEdge::kProperty, "ok");
+  CHECK_NE(NULL, ok_object);
+  const v8::HeapGraphNode* context_object =
+      GetProperty(ok_object, v8::HeapGraphEdge::kInternal, "context");
+  CHECK_NE(NULL, context_object);
+  // Check the objects are not duplicated in the context.
+  CHECK_EQ(v8::internal::Context::MIN_CONTEXT_SLOTS + num_objects - 1,
+           context_object->GetChildrenCount());
+  // Check all the objects have got their names.
+  // ... well check just every 8th because otherwise it's too slow in debug.
+  for (int i = 0; i < num_objects - 1; i += 8) {
+    char var_name[100];
+    snprintf(var_name, sizeof(var_name), "f_%d", i);
+    const v8::HeapGraphNode* f_object = GetProperty(
+        context_object, v8::HeapGraphEdge::kContextVariable, var_name);
+    CHECK_NE(NULL, f_object);
+  }
+}