From cd9be4139c9b9e13adfecd0eb027d07a62c2934b Mon Sep 17 00:00:00 2001 From: "verwaest@chromium.org" Date: Fri, 23 Aug 2013 13:21:01 +0000 Subject: [PATCH] Revert "Get rid of ConvertFieldToDescriptor and simplify related code." R=rossberg@chromium.org Review URL: https://chromiumcodereview.appspot.com/22999048 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@16297 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/objects-inl.h | 17 +++---- src/objects.cc | 144 +++++++++++++++++++++++++++--------------------------- src/objects.h | 20 +++++--- 3 files changed, 91 insertions(+), 90 deletions(-) diff --git a/src/objects-inl.h b/src/objects-inl.h index c5631b1..ad5b0e3 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -1865,15 +1865,14 @@ bool JSObject::HasFastProperties() { } -bool JSObject::TooManyFastProperties(StoreFromKeyed store_mode) { +bool JSObject::TooManyFastProperties(int properties, + JSObject::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. - Map* map = this->map(); - if (map->unused_property_fields() != 0) return false; - - 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. + int inobject = map()->inobject_properties(); int limit; if (store_mode == CERTAINLY_NOT_STORE_FROM_KEYED) { @@ -1881,7 +1880,7 @@ bool JSObject::TooManyFastProperties(StoreFromKeyed store_mode) { } else { limit = Max(inobject, kFastPropertiesSoftLimit); } - return properties()->length() > limit; + return properties > limit; } diff --git a/src/objects.cc b/src/objects.cc index 3c549d0..daa5a4a 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -1916,8 +1916,7 @@ MaybeObject* JSObject::AddFastProperty(Name* name, Object* value, PropertyAttributes attributes, StoreFromKeyed store_mode, - ValueType value_type, - TransitionFlag flag) { + ValueType value_type) { ASSERT(!IsJSGlobalProxy()); ASSERT(DescriptorArray::kNotFound == map()->instance_descriptors()->Search( @@ -1927,13 +1926,15 @@ 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()) || - TooManyFastProperties(store_mode)) { - MaybeObject* maybe_failure = + 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 = NormalizeProperties(CLEAR_INOBJECT_PROPERTIES, 0); - if (maybe_failure->IsFailure()) return maybe_failure; + if (!maybe_obj->ToObject(&obj)) return maybe_obj; + return AddSlowProperty(name, value, attributes); } @@ -1960,6 +1961,8 @@ MaybeObject* JSObject::AddFastProperty(Name* name, if (!maybe_values->To(&values)) return maybe_values; } + TransitionFlag flag = INSERT_TRANSITION; + Heap* heap = isolate->heap(); Object* storage; @@ -1992,8 +1995,7 @@ MaybeObject* JSObject::AddFastProperty(Name* name, MaybeObject* JSObject::AddConstantProperty( Name* name, Object* constant, - PropertyAttributes attributes, - TransitionFlag initial_flag) { + PropertyAttributes attributes) { // Allocate new instance descriptors with (name, constant) added ConstantDescriptor d(name, constant, attributes); @@ -2004,7 +2006,7 @@ MaybeObject* JSObject::AddConstantProperty( // attributes. attributes != NONE) ? OMIT_TRANSITION - : initial_flag; + : INSERT_TRANSITION; Map* new_map; MaybeObject* maybe_new_map = map()->CopyAddDescriptor(&d, flag); @@ -2064,8 +2066,7 @@ MaybeObject* JSObject::AddProperty(Name* name, JSReceiver::StoreFromKeyed store_mode, ExtensibilityCheck extensibility_check, ValueType value_type, - StoreMode mode, - TransitionFlag transition_flag) { + StoreMode mode) { ASSERT(!IsJSGlobalProxy()); Map* map_of_this = map(); Heap* heap = GetHeap(); @@ -2092,10 +2093,10 @@ MaybeObject* JSObject::AddProperty(Name* name, // !value->IsTheHole() && // !value->IsConsString()) { if (value->IsJSFunction()) { - result = AddConstantProperty(name, value, attributes, transition_flag); + result = AddConstantProperty(name, value, attributes); } else { result = AddFastProperty( - name, value, attributes, store_mode, value_type, transition_flag); + name, value, attributes, store_mode, value_type); } } else { // Normalize the object to prevent very large instance descriptors. @@ -2199,6 +2200,56 @@ 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"; @@ -2345,10 +2396,6 @@ 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 @@ -3749,14 +3796,8 @@ static MaybeObject* SetPropertyUsingTransition(LookupResult* lookup, PropertyDetails details = descriptors->GetDetails(descriptor); if (details.type() == CALLBACKS || attributes != details.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); + return lookup->holder()->ConvertDescriptorToField( + *name, *value, attributes); } // Keep the target CONSTANT if the same value is stored. @@ -3981,34 +4022,6 @@ 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, @@ -4087,25 +4100,10 @@ MaybeObject* JSObject::SetLocalPropertyIgnoreAttributes( if (value->IsUninitialized()) break; result = SetPropertyToField(&lookup, name, value); break; - case INTERCEPTOR: - 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); + case INTERCEPTOR: + // Override callback in clone + result = self->ConvertDescriptorToField(*name, *value, attributes); break; case TRANSITION: result = SetPropertyUsingTransition(&lookup, name, value, attributes); diff --git a/src/objects.h b/src/objects.h index 7ce339f..c75a419 100644 --- a/src/objects.h +++ b/src/objects.h @@ -2499,8 +2499,7 @@ class JSObject: public JSReceiver { MUST_USE_RESULT MaybeObject* AddConstantProperty( Name* name, Object* constant, - PropertyAttributes attributes, - TransitionFlag flag); + PropertyAttributes attributes); MUST_USE_RESULT MaybeObject* ReplaceSlowProperty( Name* name, @@ -2523,6 +2522,14 @@ 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, @@ -2535,8 +2542,7 @@ class JSObject: public JSReceiver { Object* value, PropertyAttributes attributes, StoreFromKeyed store_mode = MAY_BE_STORE_FROM_KEYED, - ValueType value_type = OPTIMAL_REPRESENTATION, - TransitionFlag flag = INSERT_TRANSITION); + ValueType value_type = OPTIMAL_REPRESENTATION); // Add a property to a slow-case object. MUST_USE_RESULT MaybeObject* AddSlowProperty(Name* name, @@ -2552,8 +2558,7 @@ 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, - TransitionFlag flag = INSERT_TRANSITION); + StoreMode mode = ALLOW_AS_CONSTANT); // Convert the object to use the canonical dictionary // representation. If the object is expected to have additional properties @@ -2689,8 +2694,7 @@ 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( - StoreFromKeyed store_mode = MAY_BE_STORE_FROM_KEYED); + inline bool TooManyFastProperties(int properties, StoreFromKeyed store_mode); // Maximal number of elements (numbered 0 .. kMaxElementCount - 1). // Also maximal value of JSArray's length property. -- 2.7.4