From 67b6605c5eaaf178e6e4e5f49d7bb90e272459bf Mon Sep 17 00:00:00 2001 From: "verwaest@chromium.org" Date: Mon, 26 Aug 2013 15:30:30 +0000 Subject: [PATCH] Get rid of ConvertFieldToDescriptor. This CL additionally fixes up the attributes for FIELD and CONSTANT in SetLocalPropertyIgnoreAttributes. R=rossberg@chromium.org Review URL: https://chromiumcodereview.appspot.com/23252008 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@16333 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/objects-inl.h | 17 ++--- src/objects.cc | 197 +++++++++++++++++++++++++++++++----------------------- src/objects.h | 21 +++--- 3 files changed, 130 insertions(+), 105 deletions(-) diff --git a/src/objects-inl.h b/src/objects-inl.h index ef0f358..22d1ac3 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -1865,14 +1865,15 @@ bool JSObject::HasFastProperties() { } -bool JSObject::TooManyFastProperties(int properties, - JSObject::StoreFromKeyed store_mode) { +bool JSObject::TooManyFastProperties(StoreFromKeyed store_mode) { // Allow extra fast properties if the object has more than - // kFastPropertiesSoftLimit in-object properties. When this is the case, - // it is very unlikely that the object is being used as a dictionary - // and there is a good chance that allowing more map transitions - // will be worth it. - int inobject = map()->inobject_properties(); + // kFastPropertiesSoftLimit in-object properties. When this is the case, it is + // very unlikely that the object is being used as a dictionary and there is a + // good chance that allowing more map transitions will be worth it. + Map* map = this->map(); + if (map->unused_property_fields() != 0) return false; + + int inobject = map->inobject_properties(); int limit; if (store_mode == CERTAINLY_NOT_STORE_FROM_KEYED) { @@ -1880,7 +1881,7 @@ bool JSObject::TooManyFastProperties(int properties, } else { limit = Max(inobject, kFastPropertiesSoftLimit); } - return properties > limit; + return properties()->length() > limit; } diff --git a/src/objects.cc b/src/objects.cc index 959e3d3..d8a05eb 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -1927,7 +1927,8 @@ MaybeObject* JSObject::AddFastProperty(Name* name, Object* value, PropertyAttributes attributes, StoreFromKeyed store_mode, - ValueType value_type) { + ValueType value_type, + TransitionFlag flag) { ASSERT(!IsJSGlobalProxy()); ASSERT(DescriptorArray::kNotFound == map()->instance_descriptors()->Search( @@ -1937,15 +1938,13 @@ MaybeObject* JSObject::AddFastProperty(Name* name, // hidden strings) and is not a real identifier. // Normalize the object if it will have too many fast properties. Isolate* isolate = GetHeap()->isolate(); - if ((!name->IsSymbol() && !IsIdentifier(isolate->unicode_cache(), name) - && name != isolate->heap()->hidden_string()) || - (map()->unused_property_fields() == 0 && - TooManyFastProperties(properties()->length(), store_mode))) { - Object* obj; - MaybeObject* maybe_obj = + if ((!name->IsSymbol() && + !IsIdentifier(isolate->unicode_cache(), name) && + name != isolate->heap()->hidden_string()) || + TooManyFastProperties(store_mode)) { + MaybeObject* maybe_failure = NormalizeProperties(CLEAR_INOBJECT_PROPERTIES, 0); - if (!maybe_obj->ToObject(&obj)) return maybe_obj; - + if (maybe_failure->IsFailure()) return maybe_failure; return AddSlowProperty(name, value, attributes); } @@ -1972,8 +1971,6 @@ MaybeObject* JSObject::AddFastProperty(Name* name, if (!maybe_values->To(&values)) return maybe_values; } - TransitionFlag flag = INSERT_TRANSITION; - Heap* heap = isolate->heap(); Object* storage; @@ -2006,7 +2003,8 @@ MaybeObject* JSObject::AddFastProperty(Name* name, MaybeObject* JSObject::AddConstantProperty( Name* name, Object* constant, - PropertyAttributes attributes) { + PropertyAttributes attributes, + TransitionFlag initial_flag) { // Allocate new instance descriptors with (name, constant) added ConstantDescriptor d(name, constant, attributes); @@ -2017,7 +2015,7 @@ MaybeObject* JSObject::AddConstantProperty( // attributes. attributes != NONE) ? OMIT_TRANSITION - : INSERT_TRANSITION; + : initial_flag; Map* new_map; MaybeObject* maybe_new_map = map()->CopyAddDescriptor(&d, flag); @@ -2077,7 +2075,8 @@ MaybeObject* JSObject::AddProperty(Name* name, JSReceiver::StoreFromKeyed store_mode, ExtensibilityCheck extensibility_check, ValueType value_type, - StoreMode mode) { + StoreMode mode, + TransitionFlag transition_flag) { ASSERT(!IsJSGlobalProxy()); Map* map_of_this = map(); Heap* heap = GetHeap(); @@ -2104,10 +2103,10 @@ MaybeObject* JSObject::AddProperty(Name* name, // !value->IsTheHole() && // !value->IsConsString()) { if (value->IsJSFunction()) { - result = AddConstantProperty(name, value, attributes); + result = AddConstantProperty(name, value, attributes, transition_flag); } else { result = AddFastProperty( - name, value, attributes, store_mode, value_type); + name, value, attributes, store_mode, value_type, transition_flag); } } else { // Normalize the object to prevent very large instance descriptors. @@ -2211,56 +2210,6 @@ MaybeObject* JSObject::ReplaceSlowProperty(Name* name, } -MaybeObject* JSObject::ConvertDescriptorToField(Name* name, - Object* new_value, - PropertyAttributes attributes, - TransitionFlag flag) { - if (map()->unused_property_fields() == 0 && - TooManyFastProperties(properties()->length(), MAY_BE_STORE_FROM_KEYED)) { - Object* obj; - MaybeObject* maybe_obj = NormalizeProperties(CLEAR_INOBJECT_PROPERTIES, 0); - if (!maybe_obj->ToObject(&obj)) return maybe_obj; - return ReplaceSlowProperty(name, new_value, attributes); - } - - Representation representation = IsJSContextExtensionObject() - ? Representation::Tagged() : new_value->OptimalRepresentation(); - int index = map()->NextFreePropertyIndex(); - FieldDescriptor new_field(name, index, attributes, representation); - - // Make a new map for the object. - Map* new_map; - MaybeObject* maybe_new_map = map()->CopyInsertDescriptor(&new_field, flag); - if (!maybe_new_map->To(&new_map)) return maybe_new_map; - - // Make new properties array if necessary. - FixedArray* new_properties = NULL; - int new_unused_property_fields = map()->unused_property_fields() - 1; - if (map()->unused_property_fields() == 0) { - new_unused_property_fields = kFieldsAdded - 1; - MaybeObject* maybe_new_properties = - properties()->CopySize(properties()->length() + kFieldsAdded); - if (!maybe_new_properties->To(&new_properties)) return maybe_new_properties; - } - - Heap* heap = GetHeap(); - Object* storage; - MaybeObject* maybe_storage = - new_value->AllocateNewStorageFor(heap, representation); - if (!maybe_storage->To(&storage)) return maybe_storage; - - // Update pointers to commit changes. - // Object points to the new map. - new_map->set_unused_property_fields(new_unused_property_fields); - set_map(new_map); - if (new_properties != NULL) { - set_properties(new_properties); - } - FastPropertyAtPut(index, storage); - return new_value; -} - - const char* Representation::Mnemonic() const { switch (kind_) { case kNone: return "v"; @@ -2407,6 +2356,10 @@ MaybeObject* JSObject::MigrateToMap(Map* new_map) { PropertyDetails details = new_descriptors->GetDetails(i); if (details.type() != FIELD) continue; PropertyDetails old_details = old_descriptors->GetDetails(i); + if (old_details.type() == CALLBACKS) { + ASSERT(details.representation().IsTagged()); + continue; + } ASSERT(old_details.type() == CONSTANT || old_details.type() == FIELD); Object* value = old_details.type() == CONSTANT @@ -2488,6 +2441,7 @@ int Map::NumberOfFields() { MaybeObject* Map::CopyGeneralizeAllRepresentations( int modify_index, StoreMode store_mode, + PropertyAttributes attributes, const char* reason) { Map* new_map; MaybeObject* maybe_map = this->Copy(); @@ -2501,7 +2455,7 @@ MaybeObject* Map::CopyGeneralizeAllRepresentations( if (store_mode == FORCE_FIELD && details.type() != FIELD) { FieldDescriptor d(descriptors->GetKey(modify_index), new_map->NumberOfFields(), - details.attributes(), + attributes, Representation::Tagged()); d.SetSortedKeyIndex(details.pointer()); descriptors->Set(modify_index, &d); @@ -2666,8 +2620,8 @@ MaybeObject* Map::GeneralizeRepresentation(int modify_index, StoreMode store_mode) { Map* old_map = this; DescriptorArray* old_descriptors = old_map->instance_descriptors(); - Representation old_representation = - old_descriptors->GetDetails(modify_index).representation(); + PropertyDetails old_details = old_descriptors->GetDetails(modify_index); + Representation old_representation = old_details.representation(); // It's fine to transition from None to anything but double without any // modification to the object, because the default uninitialized value for @@ -2686,21 +2640,22 @@ MaybeObject* Map::GeneralizeRepresentation(int modify_index, // Check the state of the root map. if (!old_map->EquivalentToForTransition(root_map)) { return CopyGeneralizeAllRepresentations( - modify_index, store_mode, "not equivalent"); + modify_index, store_mode, old_details.attributes(), "not equivalent"); } int verbatim = root_map->NumberOfOwnDescriptors(); if (store_mode != ALLOW_AS_CONSTANT && modify_index < verbatim) { return CopyGeneralizeAllRepresentations( - modify_index, store_mode, "root modification"); + modify_index, store_mode, + old_details.attributes(), "root modification"); } Map* updated = root_map->FindUpdatedMap( verbatim, descriptors, old_descriptors); if (updated == NULL) { return CopyGeneralizeAllRepresentations( - modify_index, store_mode, "incompatible"); + modify_index, store_mode, old_details.attributes(), "incompatible"); } DescriptorArray* updated_descriptors = updated->instance_descriptors(); @@ -3816,8 +3771,14 @@ static MaybeObject* SetPropertyUsingTransition(LookupResult* lookup, PropertyDetails details = descriptors->GetDetails(descriptor); if (details.type() == CALLBACKS || attributes != details.attributes()) { - return lookup->holder()->ConvertDescriptorToField( - *name, *value, attributes); + // AddProperty will either normalize the object, or create a new fast copy + // of the map. If we get a fast copy of the map, all field representations + // will be tagged since the transition is omitted. + return lookup->holder()->AddProperty( + *name, *value, attributes, kNonStrictMode, + JSReceiver::CERTAINLY_NOT_STORE_FROM_KEYED, + JSReceiver::OMIT_EXTENSIBILITY_CHECK, + JSObject::FORCE_TAGGED, FORCE_FIELD, OMIT_TRANSITION); } // Keep the target CONSTANT if the same value is stored. @@ -3882,6 +3843,56 @@ static MaybeObject* SetPropertyToField(LookupResult* lookup, } +static MaybeObject* ConvertAndSetLocalProperty(LookupResult* lookup, + Name* name, + Object* value, + PropertyAttributes attributes) { + JSObject* object = lookup->holder(); + if (object->TooManyFastProperties()) { + MaybeObject* maybe_failure = object->NormalizeProperties( + CLEAR_INOBJECT_PROPERTIES, 0); + if (maybe_failure->IsFailure()) return maybe_failure; + } + + if (!object->HasFastProperties()) { + return object->ReplaceSlowProperty(name, value, attributes); + } + + int descriptor_index = lookup->GetDescriptorIndex(); + if (lookup->GetAttributes() == attributes) { + MaybeObject* maybe_failure = object->GeneralizeFieldRepresentation( + descriptor_index, Representation::Tagged(), FORCE_FIELD); + if (maybe_failure->IsFailure()) return maybe_failure; + } else { + Map* map; + MaybeObject* maybe_map = object->map()->CopyGeneralizeAllRepresentations( + descriptor_index, FORCE_FIELD, attributes, "attributes mismatch"); + if (!maybe_map->To(&map)) return maybe_map; + MaybeObject* maybe_failure = object->MigrateToMap(map); + if (maybe_failure->IsFailure()) return maybe_failure; + } + + DescriptorArray* descriptors = object->map()->instance_descriptors(); + int index = descriptors->GetDetails(descriptor_index).field_index(); + object->FastPropertyAtPut(index, value); + return value; +} + + +static MaybeObject* SetPropertyToFieldWithAttributes( + LookupResult* lookup, + Handle name, + Handle value, + PropertyAttributes attributes) { + if (lookup->GetAttributes() == attributes) { + if (value->IsUninitialized()) return *value; + return SetPropertyToField(lookup, name, value); + } else { + return ConvertAndSetLocalProperty(lookup, *name, *value, attributes); + } +} + + MaybeObject* JSObject::SetPropertyForResult(LookupResult* lookup, Name* name_raw, Object* value_raw, @@ -4105,25 +4116,41 @@ MaybeObject* JSObject::SetLocalPropertyIgnoreAttributes( // Check of IsReadOnly removed from here in clone. MaybeObject* result = *value; switch (lookup.type()) { - case NORMAL: { - PropertyDetails details = PropertyDetails(attributes, NORMAL, 0); - result = self->SetNormalizedProperty(*name, *value, details); + case NORMAL: + result = self->ReplaceSlowProperty(*name, *value, attributes); break; - } case FIELD: - if (value->IsUninitialized()) break; - result = SetPropertyToField(&lookup, name, value); + result = SetPropertyToFieldWithAttributes( + &lookup, name, value, attributes); break; case CONSTANT: // Only replace the constant if necessary. - if (*value == lookup.GetConstant()) return *value; - if (value->IsUninitialized()) break; - result = SetPropertyToField(&lookup, name, value); + if (lookup.GetAttributes() != attributes || + *value != lookup.GetConstant()) { + result = SetPropertyToFieldWithAttributes( + &lookup, name, value, attributes); + } break; case CALLBACKS: + // Callbacks are not guaranteed to be installed on the receiver. Also + // perform a local lookup again. Fall through. case INTERCEPTOR: - // Override callback in clone - result = self->ConvertDescriptorToField(*name, *value, attributes); + self->LocalLookupRealNamedProperty(*name, &lookup); + if (lookup.IsFound()) { + if (lookup.IsPropertyCallbacks()) { + result = ConvertAndSetLocalProperty( + &lookup, *name, *value, attributes); + } else if (lookup.IsNormal()) { + result = self->ReplaceSlowProperty(*name, *value, attributes); + } else { + result = SetPropertyToFieldWithAttributes( + &lookup, name, value, attributes); + } + } else { + result = self->AddProperty( + *name, *value, attributes, kNonStrictMode, MAY_BE_STORE_FROM_KEYED, + extensibility_check, value_type, mode); + } break; case TRANSITION: result = SetPropertyUsingTransition(&lookup, name, value, attributes); diff --git a/src/objects.h b/src/objects.h index 91c3d33..411908c 100644 --- a/src/objects.h +++ b/src/objects.h @@ -2500,7 +2500,8 @@ class JSObject: public JSReceiver { MUST_USE_RESULT MaybeObject* AddConstantProperty( Name* name, Object* constant, - PropertyAttributes attributes); + PropertyAttributes attributes, + TransitionFlag flag); MUST_USE_RESULT MaybeObject* ReplaceSlowProperty( Name* name, @@ -2523,14 +2524,6 @@ class JSObject: public JSReceiver { MUST_USE_RESULT MaybeObject* TransitionElementsKind(ElementsKind to_kind); MUST_USE_RESULT MaybeObject* UpdateAllocationSite(ElementsKind to_kind); - // Converts a descriptor of any other type to a real field, backed by the - // properties array. - MUST_USE_RESULT MaybeObject* ConvertDescriptorToField( - Name* name, - Object* new_value, - PropertyAttributes attributes, - TransitionFlag flag = OMIT_TRANSITION); - MUST_USE_RESULT MaybeObject* MigrateToMap(Map* new_map); MUST_USE_RESULT MaybeObject* GeneralizeFieldRepresentation( int modify_index, @@ -2543,7 +2536,8 @@ class JSObject: public JSReceiver { Object* value, PropertyAttributes attributes, StoreFromKeyed store_mode = MAY_BE_STORE_FROM_KEYED, - ValueType value_type = OPTIMAL_REPRESENTATION); + ValueType value_type = OPTIMAL_REPRESENTATION, + TransitionFlag flag = INSERT_TRANSITION); // Add a property to a slow-case object. MUST_USE_RESULT MaybeObject* AddSlowProperty(Name* name, @@ -2559,7 +2553,8 @@ class JSObject: public JSReceiver { StoreFromKeyed store_mode = MAY_BE_STORE_FROM_KEYED, ExtensibilityCheck extensibility_check = PERFORM_EXTENSIBILITY_CHECK, ValueType value_type = OPTIMAL_REPRESENTATION, - StoreMode mode = ALLOW_AS_CONSTANT); + StoreMode mode = ALLOW_AS_CONSTANT, + TransitionFlag flag = INSERT_TRANSITION); // Convert the object to use the canonical dictionary // representation. If the object is expected to have additional properties @@ -2695,7 +2690,8 @@ class JSObject: public JSReceiver { // 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. - inline bool TooManyFastProperties(int properties, StoreFromKeyed store_mode); + inline bool TooManyFastProperties( + StoreFromKeyed store_mode = MAY_BE_STORE_FROM_KEYED); // Maximal number of elements (numbered 0 .. kMaxElementCount - 1). // Also maximal value of JSArray's length property. @@ -5631,6 +5627,7 @@ class Map: public HeapObject { MUST_USE_RESULT MaybeObject* CopyGeneralizeAllRepresentations( int modify_index, StoreMode store_mode, + PropertyAttributes attributes, const char* reason); void PrintGeneralization(FILE* file, -- 2.7.4