From 1afc70a3327494f0bdaf18bc96c98ada0a6c287f Mon Sep 17 00:00:00 2001 From: "alph@chromium.org" Date: Wed, 13 Mar 2013 17:38:04 +0000 Subject: [PATCH] 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 --- include/v8-profiler.h | 3 ++- src/heap-snapshot-generator.cc | 53 +++++++++++++++++++++------------------ src/heap-snapshot-generator.h | 8 +++--- test/cctest/test-heap-profiler.cc | 43 +++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 28 deletions(-) diff --git a/include/v8-profiler.h b/include/v8-profiler.h index 4d3597a..c9eab3f 100644 --- 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 --- 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 --- 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 --- 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); + } +} -- 2.7.4