From 08146dc023ea02608a4184f88c292bd8f299aed0 Mon Sep 17 00:00:00 2001 From: ishell Date: Tue, 16 Dec 2014 05:22:23 -0800 Subject: [PATCH] Introduced PropertyType ACCESSOR_FIELD. Review URL: https://codereview.chromium.org/805453002 Cr-Commit-Position: refs/heads/master@{#25842} --- src/bootstrapper.cc | 3 ++ src/factory.cc | 2 +- src/heap-snapshot-generator.cc | 89 +++++++++++++++---------------- src/heap-snapshot-generator.h | 10 +++- src/lookup-inl.h | 7 ++- src/objects-printer.cc | 27 +++++----- src/objects.cc | 48 ++++++++++------- src/property-details.h | 1 + src/property.cc | 37 +++++-------- test/cctest/test-heap-profiler.cc | 41 ++++++++++++++ test/cctest/test-transitions.cc | 18 +++++++ 11 files changed, 176 insertions(+), 107 deletions(-) diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index 60118487bc..7105eb25e2 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -2581,6 +2581,8 @@ void Genesis::TransferNamedProperties(Handle from, JSObject::AddProperty(to, key, constant, details.attributes()); break; } + case ACCESSOR_FIELD: + UNREACHABLE(); case CALLBACKS: { Handle key(descs->GetKey(i)); LookupIterator it(to, key, LookupIterator::OWN_SKIP_INTERCEPTOR); @@ -2619,6 +2621,7 @@ void Genesis::TransferNamedProperties(Handle from, isolate()); } PropertyDetails details = properties->DetailsAt(i); + DCHECK_EQ(DATA, details.kind()); JSObject::AddProperty(to, key, value, details.attributes()); } } diff --git a/src/factory.cc b/src/factory.cc index 757f4fffc2..83e5a440d6 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -1592,7 +1592,7 @@ Handle Factory::NewGlobalObject(Handle constructor) { for (int i = 0; i < map->NumberOfOwnDescriptors(); i++) { PropertyDetails details = descs->GetDetails(i); DCHECK(details.type() == CALLBACKS); // Only accessors are expected. - PropertyDetails d = PropertyDetails(details.attributes(), CALLBACKS, i + 1); + PropertyDetails d(details.attributes(), CALLBACKS, i + 1); Handle name(descs->GetKey(i)); Handle value(descs->GetCallbacksObject(i), isolate()); Handle cell = NewPropertyCell(value); diff --git a/src/heap-snapshot-generator.cc b/src/heap-snapshot-generator.cc index b7b290e363..196eb130a5 100644 --- a/src/heap-snapshot-generator.cc +++ b/src/heap-snapshot-generator.cc @@ -1626,50 +1626,32 @@ void V8HeapExplorer::ExtractPropertyReferences(JSObject* js_obj, int entry) { DescriptorArray* descs = js_obj->map()->instance_descriptors(); int real_size = js_obj->map()->NumberOfOwnDescriptors(); for (int i = 0; i < real_size; i++) { - switch (descs->GetType(i)) { - case FIELD: { - Representation r = descs->GetDetails(i).representation(); + PropertyDetails details = descs->GetDetails(i); + switch (details.location()) { + case IN_OBJECT: { + Representation r = details.representation(); if (r.IsSmi() || r.IsDouble()) break; - int index = descs->GetFieldIndex(i); Name* k = descs->GetKey(i); - if (index < js_obj->map()->inobject_properties()) { - Object* value = js_obj->InObjectPropertyAt(index); - if (k != heap_->hidden_string()) { - SetPropertyReference( - js_obj, entry, - k, value, - NULL, - js_obj->GetInObjectPropertyOffset(index)); - } else { - TagObject(value, "(hidden properties)"); - SetInternalReference( - js_obj, entry, - "hidden_properties", value, - js_obj->GetInObjectPropertyOffset(index)); - } + FieldIndex field_index = FieldIndex::ForDescriptor(js_obj->map(), i); + Object* value = js_obj->RawFastPropertyAt(field_index); + int field_offset = + field_index.is_inobject() ? field_index.offset() : -1; + + if (k != heap_->hidden_string()) { + SetDataOrAccessorPropertyReference(details.kind(), js_obj, entry, k, + value, NULL, field_offset); } else { - FieldIndex field_index = - FieldIndex::ForDescriptor(js_obj->map(), i); - Object* value = js_obj->RawFastPropertyAt(field_index); - if (k != heap_->hidden_string()) { - SetPropertyReference(js_obj, entry, k, value); - } else { - TagObject(value, "(hidden properties)"); - SetInternalReference(js_obj, entry, "hidden_properties", value); - } + TagObject(value, "(hidden properties)"); + SetInternalReference(js_obj, entry, "hidden_properties", value, + field_offset); } break; } - case CONSTANT: - SetPropertyReference( - js_obj, entry, - descs->GetKey(i), descs->GetConstant(i)); - break; - case CALLBACKS: - ExtractAccessorPairProperty( - js_obj, entry, - descs->GetKey(i), descs->GetValue(i)); + case IN_DESCRIPTOR: + SetDataOrAccessorPropertyReference(details.kind(), js_obj, entry, + descs->GetKey(i), + descs->GetValue(i)); break; } } @@ -1689,27 +1671,30 @@ void V8HeapExplorer::ExtractPropertyReferences(JSObject* js_obj, int entry) { SetInternalReference(js_obj, entry, "hidden_properties", value); continue; } - if (ExtractAccessorPairProperty(js_obj, entry, k, value)) continue; - SetPropertyReference(js_obj, entry, Name::cast(k), value); + PropertyDetails details = dictionary->DetailsAt(i); + SetDataOrAccessorPropertyReference(details.kind(), js_obj, entry, + Name::cast(k), value); } } } } -bool V8HeapExplorer::ExtractAccessorPairProperty( - JSObject* js_obj, int entry, Object* key, Object* callback_obj) { - if (!callback_obj->IsAccessorPair()) return false; +void V8HeapExplorer::ExtractAccessorPairProperty(JSObject* js_obj, int entry, + Name* key, + Object* callback_obj, + int field_offset) { + if (!callback_obj->IsAccessorPair()) return; AccessorPair* accessors = AccessorPair::cast(callback_obj); + SetPropertyReference(js_obj, entry, key, accessors, NULL, field_offset); Object* getter = accessors->getter(); if (!getter->IsOddball()) { - SetPropertyReference(js_obj, entry, Name::cast(key), getter, "get %s"); + SetPropertyReference(js_obj, entry, key, getter, "get %s"); } Object* setter = accessors->setter(); if (!setter->IsOddball()) { - SetPropertyReference(js_obj, entry, Name::cast(key), setter, "set %s"); + SetPropertyReference(js_obj, entry, key, setter, "set %s"); } - return true; } @@ -2048,6 +2033,20 @@ void V8HeapExplorer::SetWeakReference(HeapObject* parent_obj, } +void V8HeapExplorer::SetDataOrAccessorPropertyReference( + PropertyKind kind, JSObject* parent_obj, int parent_entry, + Name* reference_name, Object* child_obj, const char* name_format_string, + int field_offset) { + if (kind == ACCESSOR) { + ExtractAccessorPairProperty(parent_obj, parent_entry, reference_name, + child_obj, field_offset); + } else { + SetPropertyReference(parent_obj, parent_entry, reference_name, child_obj, + name_format_string, field_offset); + } +} + + void V8HeapExplorer::SetPropertyReference(HeapObject* parent_obj, int parent_entry, Name* reference_name, diff --git a/src/heap-snapshot-generator.h b/src/heap-snapshot-generator.h index fb4387617d..8aef43739c 100644 --- a/src/heap-snapshot-generator.h +++ b/src/heap-snapshot-generator.h @@ -381,8 +381,8 @@ class V8HeapExplorer : public HeapEntriesAllocator { void ExtractFixedArrayReferences(int entry, FixedArray* array); void ExtractClosureReferences(JSObject* js_obj, int entry); void ExtractPropertyReferences(JSObject* js_obj, int entry); - bool ExtractAccessorPairProperty(JSObject* js_obj, int entry, - Object* key, Object* callback_obj); + void ExtractAccessorPairProperty(JSObject* js_obj, int entry, Name* key, + Object* callback_obj, int field_offset = -1); void ExtractElementReferences(JSObject* js_obj, int entry); void ExtractInternalReferences(JSObject* js_obj, int entry); @@ -430,6 +430,12 @@ class V8HeapExplorer : public HeapEntriesAllocator { Object* child, const char* name_format_string = NULL, int field_offset = -1); + void SetDataOrAccessorPropertyReference(PropertyKind kind, + JSObject* parent_obj, int parent, + Name* reference_name, Object* child, + const char* name_format_string = NULL, + int field_offset = -1); + void SetUserGlobalReference(Object* user_global); void SetRootGcRootsReference(); void SetGcRootsReference(VisitorSynchronization::SyncTag tag); diff --git a/src/lookup-inl.h b/src/lookup-inl.h index 1adf227496..0c9cc91b2f 100644 --- a/src/lookup-inl.h +++ b/src/lookup-inl.h @@ -63,11 +63,10 @@ LookupIterator::State LookupIterator::LookupInHolder(Map* map, property_details_ = descriptors->GetDetails(number_); } has_property_ = true; - switch (property_details_.type()) { - case v8::internal::CONSTANT: - case v8::internal::FIELD: + switch (property_details_.kind()) { + case v8::internal::DATA: return DATA; - case v8::internal::CALLBACKS: + case v8::internal::ACCESSOR: return ACCESSOR; } case ACCESSOR: diff --git a/src/objects-printer.cc b/src/objects-printer.cc index fc72800d16..9805490c31 100644 --- a/src/objects-printer.cc +++ b/src/objects-printer.cc @@ -241,11 +241,16 @@ void JSObject::PrintProperties(std::ostream& os) { // NOLINT os << " (field at offset " << index.property_index() << ")\n"; break; } + case ACCESSOR_FIELD: { + FieldIndex index = FieldIndex::ForDescriptor(map(), i); + os << " (accessor at offset " << index.property_index() << ")\n"; + break; + } case CONSTANT: os << Brief(descs->GetConstant(i)) << " (constant)\n"; break; case CALLBACKS: - os << Brief(descs->GetCallbacksObject(i)) << " (callback)\n"; + os << Brief(descs->GetCallbacksObject(i)) << " (callbacks)\n"; break; } } @@ -1189,19 +1194,15 @@ void TransitionArray::PrintTransitions(std::ostream& os, os << " (transition to Object.observe)"; } else { PropertyDetails details = GetTargetDetails(key, target); - switch (details.type()) { - case FIELD: { - os << " (transition to field)"; - break; - } - case CONSTANT: - os << " (transition to constant " << Brief(GetTargetValue(i)) << ")"; - break; - case CALLBACKS: - os << " (transition to callback " << Brief(GetTargetValue(i)) << ")"; - break; + os << " (transition to "; + if (details.location() == IN_DESCRIPTOR) { + os << "immutable "; } - os << ", attrs: " << details.attributes(); + os << (details.kind() == DATA ? "data" : "accessor"); + if (details.location() == IN_DESCRIPTOR) { + os << " " << Brief(GetTargetValue(i)); + } + os << "), attrs: " << details.attributes(); } os << " -> " << Brief(target) << "\n"; } diff --git a/src/objects.cc b/src/objects.cc index 6b5d80c992..9f9f931df0 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -2866,29 +2866,35 @@ MaybeHandle Map::TryUpdateInternal(Handle old_map) { DescriptorArray* new_descriptors = new_map->instance_descriptors(); PropertyDetails new_details = new_descriptors->GetDetails(i); - if (old_details.attributes() != new_details.attributes() || - !old_details.representation().fits_into(new_details.representation())) { + DCHECK_EQ(old_details.kind(), new_details.kind()); + DCHECK_EQ(old_details.attributes(), new_details.attributes()); + if (!old_details.representation().fits_into(new_details.representation())) { return MaybeHandle(); } - PropertyType new_type = new_details.type(); - PropertyType old_type = old_details.type(); Object* new_value = new_descriptors->GetValue(i); Object* old_value = old_descriptors->GetValue(i); - switch (new_type) { - case FIELD: - if ((old_type == FIELD && - !HeapType::cast(old_value)->NowIs(HeapType::cast(new_value))) || - (old_type == CONSTANT && - !HeapType::cast(new_value)->NowContains(old_value)) || - (old_type == CALLBACKS && - !HeapType::Any()->Is(HeapType::cast(new_value)))) { - return MaybeHandle(); + switch (new_details.type()) { + case FIELD: { + PropertyType old_type = old_details.type(); + if (old_type == FIELD) { + if (!HeapType::cast(old_value)->NowIs(HeapType::cast(new_value))) { + return MaybeHandle(); + } + } else { + DCHECK(old_type == CONSTANT); + if (!HeapType::cast(new_value)->NowContains(old_value)) { + return MaybeHandle(); + } } break; + } + case ACCESSOR_FIELD: + DCHECK(HeapType::Any()->Is(HeapType::cast(new_value))); + break; case CONSTANT: case CALLBACKS: - if (old_type != new_type || old_value != new_value) { + if (old_details.location() == IN_OBJECT || old_value != new_value) { return MaybeHandle(); } break; @@ -4364,16 +4370,15 @@ void JSObject::MigrateFastToSlow(Handle object, Handle descs(map->instance_descriptors()); for (int i = 0; i < real_size; i++) { PropertyDetails details = descs->GetDetails(i); + Handle key(descs->GetKey(i)); switch (details.type()) { case CONSTANT: { - Handle key(descs->GetKey(i)); Handle value(descs->GetConstant(i), isolate); PropertyDetails d(details.attributes(), FIELD, i + 1); dictionary = NameDictionary::Add(dictionary, key, value, d); break; } case FIELD: { - Handle key(descs->GetKey(i)); FieldIndex index = FieldIndex::ForDescriptor(*map, i); Handle value; if (object->IsUnboxedDoubleField(index)) { @@ -4391,8 +4396,14 @@ void JSObject::MigrateFastToSlow(Handle object, dictionary = NameDictionary::Add(dictionary, key, value, d); break; } + case ACCESSOR_FIELD: { + FieldIndex index = FieldIndex::ForDescriptor(*map, i); + Handle value(object->RawFastPropertyAt(index), isolate); + PropertyDetails d(details.attributes(), CALLBACKS, i + 1); + dictionary = NameDictionary::Add(dictionary, key, value, d); + break; + } case CALLBACKS: { - Handle key(descs->GetKey(i)); Handle value(descs->GetCallbacksObject(i), isolate); PropertyDetails d(details.attributes(), CALLBACKS, i + 1); dictionary = NameDictionary::Add(dictionary, key, value, d); @@ -7069,6 +7080,7 @@ bool DescriptorArray::CanHoldValue(int descriptor, Object* value) { value->FitsRepresentation(details.representation())); return GetConstant(descriptor) == value; + case ACCESSOR_FIELD: case CALLBACKS: return false; } @@ -7194,7 +7206,7 @@ Handle Map::TransitionToAccessorProperty(Handle map, int descriptor = transition->LastAdded(); DCHECK(descriptors->GetKey(descriptor)->Equals(*name)); - DCHECK_EQ(CALLBACKS, descriptors->GetDetails(descriptor).type()); + DCHECK_EQ(ACCESSOR, descriptors->GetDetails(descriptor).kind()); DCHECK_EQ(attributes, descriptors->GetDetails(descriptor).attributes()); Handle maybe_pair(descriptors->GetValue(descriptor), isolate); diff --git a/src/property-details.h b/src/property-details.h index 11fe52c100..6140e0d4bb 100644 --- a/src/property-details.h +++ b/src/property-details.h @@ -57,6 +57,7 @@ enum PropertyLocation { IN_OBJECT = 0, IN_DESCRIPTOR = 1 }; enum PropertyType { FIELD = (IN_OBJECT << 1) | DATA, CONSTANT = (IN_DESCRIPTOR << 1) | DATA, + ACCESSOR_FIELD = (IN_OBJECT << 1) | ACCESSOR, CALLBACKS = (IN_DESCRIPTOR << 1) | ACCESSOR }; diff --git a/src/property.cc b/src/property.cc index 512527a5de..d00998c256 100644 --- a/src/property.cc +++ b/src/property.cc @@ -51,18 +51,11 @@ struct FastPropertyDetails { // Outputs PropertyDetails as a dictionary details. std::ostream& operator<<(std::ostream& os, const PropertyDetails& details) { os << "("; - switch (details.type()) { - case FIELD: - os << "normal: "; - break; - case CALLBACKS: - os << "callbacks: "; - break; - case CONSTANT: - UNREACHABLE(); - break; + if (details.location() == IN_DESCRIPTOR) { + os << "immutable "; } - return os << " dictionary_index: " << details.dictionary_index() + os << (details.kind() == DATA ? "data" : "accessor"); + return os << ", dictionary_index: " << details.dictionary_index() << ", attrs: " << details.attributes() << ")"; } @@ -72,20 +65,16 @@ std::ostream& operator<<(std::ostream& os, const FastPropertyDetails& details_fast) { const PropertyDetails& details = details_fast.details; os << "("; - switch (details.type()) { - case CONSTANT: - os << "constant: p: " << details.pointer(); - break; - case FIELD: - os << "field: " << details.representation().Mnemonic() - << ", field_index: " << details.field_index() - << ", p: " << details.pointer(); - break; - case CALLBACKS: - os << "callbacks: p: " << details.pointer(); - break; + if (details.location() == IN_DESCRIPTOR) { + os << "immutable "; } - return os << ", attrs: " << details.attributes() << ")"; + os << (details.kind() == DATA ? "data" : "accessor"); + if (details.location() == IN_OBJECT) { + os << ": " << details.representation().Mnemonic() + << ", field_index: " << details.field_index(); + } + return os << ", p: " << details.pointer() + << ", attrs: " << details.attributes() << ")"; } diff --git a/test/cctest/test-heap-profiler.cc b/test/cctest/test-heap-profiler.cc index 203b174746..94a5be47c1 100644 --- a/test/cctest/test-heap-profiler.cc +++ b/test/cctest/test-heap-profiler.cc @@ -1916,6 +1916,47 @@ TEST(FastCaseAccessors) { } +TEST(FastCaseRedefinedAccessors) { + LocalContext env; + v8::HandleScope scope(env->GetIsolate()); + v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler(); + + CompileRun( + "var obj1 = {};\n" + "Object.defineProperty(obj1, 'prop', { " + " get: function() { return 42; },\n" + " set: function(value) { return this.prop_ = value; },\n" + " configurable: true,\n" + " enumerable: true,\n" + "});\n" + "Object.defineProperty(obj1, 'prop', { " + " get: function() { return 153; },\n" + " set: function(value) { return this.prop_ = value; },\n" + " configurable: true,\n" + " enumerable: true,\n" + "});\n"); + v8::Local js_global = + env->Global()->GetPrototype().As(); + i::Handle js_obj1 = + v8::Utils::OpenHandle(*js_global->Get(v8_str("obj1")).As()); + USE(js_obj1); + + const v8::HeapSnapshot* snapshot = + heap_profiler->TakeHeapSnapshot(v8_str("fastCaseAccessors")); + CHECK(ValidateSnapshot(snapshot)); + const v8::HeapGraphNode* global = GetGlobalObject(snapshot); + CHECK_NE(NULL, global); + const v8::HeapGraphNode* obj1 = + GetProperty(global, v8::HeapGraphEdge::kProperty, "obj1"); + CHECK_NE(NULL, obj1); + const v8::HeapGraphNode* func; + func = GetProperty(obj1, v8::HeapGraphEdge::kProperty, "get prop"); + CHECK_NE(NULL, func); + func = GetProperty(obj1, v8::HeapGraphEdge::kProperty, "set prop"); + CHECK_NE(NULL, func); +} + + TEST(SlowCaseAccessors) { LocalContext env; v8::HandleScope scope(env->GetIsolate()); diff --git a/test/cctest/test-transitions.cc b/test/cctest/test-transitions.cc index 981461326c..6bcdb35e7e 100644 --- a/test/cctest/test-transitions.cc +++ b/test/cctest/test-transitions.cc @@ -30,6 +30,24 @@ static void ConnectTransition(Handle parent, } +static void CheckPropertyDetailsFieldsConsistency(PropertyType type, + PropertyKind kind, + PropertyLocation location) { + int type_value = PropertyDetails::TypeField::encode(type); + int kind_location_value = PropertyDetails::KindField::encode(kind) | + PropertyDetails::LocationField::encode(location); + CHECK_EQ(type_value, kind_location_value); +} + + +TEST(PropertyDetailsFieldsConsistency) { + CheckPropertyDetailsFieldsConsistency(FIELD, DATA, IN_OBJECT); + CheckPropertyDetailsFieldsConsistency(CONSTANT, DATA, IN_DESCRIPTOR); + CheckPropertyDetailsFieldsConsistency(ACCESSOR_FIELD, ACCESSOR, IN_OBJECT); + CheckPropertyDetailsFieldsConsistency(CALLBACKS, ACCESSOR, IN_DESCRIPTOR); +} + + TEST(TransitionArray_SimpleFieldTransitions) { CcTest::InitializeVM(); v8::HandleScope scope(CcTest::isolate());