From f457809c088155662dee5ca61b0c2699f9685807 Mon Sep 17 00:00:00 2001 From: "verwaest@chromium.org" Date: Fri, 23 Aug 2013 11:52:59 +0000 Subject: [PATCH] Get rid of ConvertFieldToDescriptor and simplify related code. R=rossberg@chromium.org Review URL: https://chromiumcodereview.appspot.com/22861025 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@16295 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/objects-inl.h | 17 ++++--- src/objects.cc | 144 +++++++++++++++++++++++++++--------------------------- src/objects.h | 20 +++----- 3 files changed, 90 insertions(+), 91 deletions(-) diff --git a/src/objects-inl.h b/src/objects-inl.h index ad5b0e3..c5631b1 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 daa5a4a..3c549d0 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -1916,7 +1916,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( @@ -1926,15 +1927,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); } @@ -1961,8 +1960,6 @@ MaybeObject* JSObject::AddFastProperty(Name* name, if (!maybe_values->To(&values)) return maybe_values; } - TransitionFlag flag = INSERT_TRANSITION; - Heap* heap = isolate->heap(); Object* storage; @@ -1995,7 +1992,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); @@ -2006,7 +2004,7 @@ MaybeObject* JSObject::AddConstantProperty( // attributes. attributes != NONE) ? OMIT_TRANSITION - : INSERT_TRANSITION; + : initial_flag; Map* new_map; MaybeObject* maybe_new_map = map()->CopyAddDescriptor(&d, flag); @@ -2066,7 +2064,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(); @@ -2093,10 +2092,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. @@ -2200,56 +2199,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"; @@ -2396,6 +2345,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 @@ -3796,8 +3749,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. @@ -4022,6 +3981,34 @@ Handle JSObject::SetLocalPropertyIgnoreAttributes( } +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(); + MaybeObject* maybe_failure = object->GeneralizeFieldRepresentation( + descriptor_index, Representation::Tagged(), FORCE_FIELD); + 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; +} + + MaybeObject* JSObject::SetLocalPropertyIgnoreAttributes( Name* name_raw, Object* value_raw, @@ -4100,10 +4087,25 @@ MaybeObject* JSObject::SetLocalPropertyIgnoreAttributes( if (value->IsUninitialized()) break; result = SetPropertyToField(&lookup, name, value); break; - case CALLBACKS: 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 = SetPropertyToField(&lookup, name, value); + } + } else { + result = self->AddProperty( + *name, *value, attributes, kNonStrictMode, MAY_BE_STORE_FROM_KEYED, + extensibility_check, value_type, mode); + } + break; + case CALLBACKS: + result = ConvertAndSetLocalProperty(&lookup, *name, *value, attributes); break; case TRANSITION: result = SetPropertyUsingTransition(&lookup, name, value, attributes); diff --git a/src/objects.h b/src/objects.h index c75a419..7ce339f 100644 --- a/src/objects.h +++ b/src/objects.h @@ -2499,7 +2499,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, @@ -2522,14 +2523,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, @@ -2542,7 +2535,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, @@ -2558,7 +2552,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 @@ -2694,7 +2689,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. -- 2.7.4