From 007ad200f82a789e920c32281b53812f6b34bb62 Mon Sep 17 00:00:00 2001 From: "mikhail.naganov@gmail.com" Date: Mon, 14 Nov 2011 11:13:29 +0000 Subject: [PATCH] Fix missing fast property accessors in heap snapshots. Implementation for this case var x = {}; x.__defineGetter__("y", function Y() { return 42; }); BUG=v8:1818 TEST=cctest/test-heap-profiler/FastCaseGetter Review URL: http://codereview.chromium.org/8491041 Patch from Ilya Tikhonovsky . git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9985 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/objects.cc | 12 +++++++----- src/objects.h | 5 +++++ src/profile-generator.cc | 30 ++++++++++++++++++++++++++---- src/profile-generator.h | 1 + test/cctest/test-heap-profiler.cc | 27 +++++++++++++++++++++++++++ 5 files changed, 66 insertions(+), 9 deletions(-) diff --git a/src/objects.cc b/src/objects.cc index ca8714a63d..88efc3c181 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -103,11 +103,6 @@ void PrintElementsKind(FILE* out, ElementsKind kind) { } -// Getters and setters are stored in a fixed array property. These are -// constants for their indices. -const int kGetterIndex = 0; -const int kSetterIndex = 1; - MUST_USE_RESULT static MaybeObject* CreateJSValue(JSFunction* constructor, Object* value) { Object* result; @@ -210,6 +205,13 @@ MaybeObject* Object::GetPropertyWithReceiver(Object* receiver, } +// This may seem strange but the standard requires inline static const +// definition, and w/o these the code doesn't link when being built in debug +// mode using gcc. +const int JSObject::kGetterIndex; +const int JSObject::kSetterIndex; + + MaybeObject* JSObject::GetPropertyWithCallback(Object* receiver, Object* structure, String* name) { diff --git a/src/objects.h b/src/objects.h index a7554d28be..a592a33694 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1886,6 +1886,11 @@ class JSObject: public JSReceiver { #endif Object* SlowReverseLookup(Object* value); + // Getters and setters are stored in a fixed array property. + // These are constants for their indices. + static const int kGetterIndex = 0; + static const int kSetterIndex = 1; + // Maximal number of fast properties for the JSObject. Used to // restrict the number of map transitions to avoid an explosion in // the number of maps for objects used as dictionaries. diff --git a/src/profile-generator.cc b/src/profile-generator.cc index 8a68f5326b..1a2d005a6f 100644 --- a/src/profile-generator.cc +++ b/src/profile-generator.cc @@ -1918,6 +1918,7 @@ void V8HeapExplorer::ExtractReferences(HeapObject* obj) { SetPropertyReference( obj, entry, heap_->prototype_symbol(), proto_or_map, + NULL, JSFunction::kPrototypeOrInitialMapOffset); } else { SetPropertyReference( @@ -2101,6 +2102,7 @@ void V8HeapExplorer::ExtractPropertyReferences(JSObject* js_obj, SetPropertyReference( js_obj, entry, descs->GetKey(i), js_obj->InObjectPropertyAt(index), + NULL, js_obj->GetInObjectPropertyOffset(index)); } else { SetPropertyReference( @@ -2114,6 +2116,21 @@ void V8HeapExplorer::ExtractPropertyReferences(JSObject* js_obj, js_obj, entry, descs->GetKey(i), descs->GetConstantFunction(i)); break; + case CALLBACKS: { + Object* callback_obj = descs->GetValue(i); + if (callback_obj->IsFixedArray()) { + FixedArray* accessors = FixedArray::cast(callback_obj); + if (Object* getter = accessors->get(JSObject::kGetterIndex)) { + SetPropertyReference(js_obj, entry, descs->GetKey(i), + getter, "get-%s"); + } + if (Object* setter = accessors->get(JSObject::kSetterIndex)) { + SetPropertyReference(js_obj, entry, descs->GetKey(i), + setter, "set-%s"); + } + } + break; + } case NORMAL: // only in slow mode case HANDLER: // only in lookup results, not in descriptors case INTERCEPTOR: // only in lookup results, not in descriptors @@ -2122,9 +2139,6 @@ void V8HeapExplorer::ExtractPropertyReferences(JSObject* js_obj, case CONSTANT_TRANSITION: case NULL_DESCRIPTOR: // ... and not about "holes" break; - // TODO(svenpanne): Should we really ignore accessors here? - case CALLBACKS: - break; } } } else { @@ -2349,15 +2363,23 @@ void V8HeapExplorer::SetPropertyReference(HeapObject* parent_obj, HeapEntry* parent_entry, String* reference_name, Object* child_obj, + const char* name_format_string, int field_offset) { HeapEntry* child_entry = GetEntry(child_obj); if (child_entry != NULL) { HeapGraphEdge::Type type = reference_name->length() > 0 ? HeapGraphEdge::kProperty : HeapGraphEdge::kInternal; + const char* name = name_format_string != NULL ? + collection_->names()->GetFormatted( + name_format_string, + *reference_name->ToCString(DISALLOW_NULLS, + ROBUST_STRING_TRAVERSAL)) : + collection_->names()->GetName(reference_name); + filler_->SetNamedReference(type, parent_obj, parent_entry, - collection_->names()->GetName(reference_name), + name, child_obj, child_entry); IndexedReferencesExtractor::MarkVisitedField(parent_obj, field_offset); diff --git a/src/profile-generator.h b/src/profile-generator.h index 5747dbd5e9..44be3db78c 100644 --- a/src/profile-generator.h +++ b/src/profile-generator.h @@ -973,6 +973,7 @@ class V8HeapExplorer : public HeapEntriesAllocator { HeapEntry* parent, String* reference_name, Object* child, + const char* name_format_string = NULL, int field_offset = -1); void SetPropertyShortcutReference(HeapObject* parent_obj, HeapEntry* parent, diff --git a/test/cctest/test-heap-profiler.cc b/test/cctest/test-heap-profiler.cc index 87e7a7d0f9..f835a1e0d5 100644 --- a/test/cctest/test-heap-profiler.cc +++ b/test/cctest/test-heap-profiler.cc @@ -1023,3 +1023,30 @@ TEST(GetConstructorName) { CHECK_EQ(0, StringCmp( "Object", i::V8HeapExplorer::GetConstructorName(*js_obj6))); } + +TEST(FastCaseGetter) { + v8::HandleScope scope; + LocalContext env; + + CompileRun("var obj1 = {};\n" + "obj1.__defineGetter__('propWithGetter', function Y() {\n" + " return 42;\n" + "});\n" + "obj1.__defineSetter__('propWithSetter', function Z(value) {\n" + " return this.value_ = value;\n" + "});\n"); + const v8::HeapSnapshot* snapshot = + v8::HeapProfiler::TakeSnapshot(v8_str("fastCaseGetter")); + + const v8::HeapGraphNode* global = GetGlobalObject(snapshot); + CHECK_NE(NULL, global); + const v8::HeapGraphNode* obj1 = + GetProperty(global, v8::HeapGraphEdge::kShortcut, "obj1"); + CHECK_NE(NULL, obj1); + const v8::HeapGraphNode* getterFunction = + GetProperty(obj1, v8::HeapGraphEdge::kProperty, "get-propWithGetter"); + CHECK_NE(NULL, getterFunction); + const v8::HeapGraphNode* setterFunction = + GetProperty(obj1, v8::HeapGraphEdge::kProperty, "set-propWithSetter"); + CHECK_NE(NULL, setterFunction); +}