From 648e99e3087221f919065201a3be18dab830793f Mon Sep 17 00:00:00 2001 From: "adamk@chromium.org" Date: Wed, 22 May 2013 18:53:58 +0000 Subject: [PATCH] Make Object.freeze fast This patch both speeds up the freeze operation itself, but also allows properties to remain in fast mode. Objects with non-empty elements backing stores still end up with slow elements. BUG=v8:1858, 115960 R=verwaest@chromium.org Review URL: https://codereview.chromium.org/14888005 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@14758 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- include/v8.h | 2 +- src/heap.cc | 11 ++ src/heap.h | 5 +- src/objects-inl.h | 10 ++ src/objects.cc | 269 ++++++++++++++++++++++++++++++++---------- src/objects.h | 24 +++- src/property-details.h | 10 +- src/runtime.cc | 9 ++ src/runtime.h | 4 + src/v8natives.js | 31 +++-- test/mjsunit/object-freeze.js | 62 ++++++++++ 11 files changed, 355 insertions(+), 82 deletions(-) diff --git a/include/v8.h b/include/v8.h index 37bd427..696892b 100644 --- a/include/v8.h +++ b/include/v8.h @@ -5231,7 +5231,7 @@ class Internals { static const int kNullValueRootIndex = 7; static const int kTrueValueRootIndex = 8; static const int kFalseValueRootIndex = 9; - static const int kEmptyStringRootIndex = 127; + static const int kEmptyStringRootIndex = 129; static const int kNodeClassIdOffset = 1 * kApiPointerSize; static const int kNodeFlagsOffset = 1 * kApiPointerSize + 3; diff --git a/src/heap.cc b/src/heap.cc index 98844f0..424edd4 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -2964,6 +2964,17 @@ bool Heap::CreateInitialObjects() { } set_observation_state(JSObject::cast(obj)); + { MaybeObject* maybe_obj = AllocateSymbol(); + if (!maybe_obj->ToObject(&obj)) return false; + } + set_frozen_symbol(Symbol::cast(obj)); + + { MaybeObject* maybe_obj = SeededNumberDictionary::Allocate(this, 0, TENURED); + if (!maybe_obj->ToObject(&obj)) return false; + } + SeededNumberDictionary::cast(obj)->set_requires_slow_elements(); + set_empty_slow_element_dictionary(SeededNumberDictionary::cast(obj)); + // Handling of script id generation is in FACTORY->NewScript. set_last_script_id(undefined_value()); diff --git a/src/heap.h b/src/heap.h index b24b0b3..99b14bd 100644 --- a/src/heap.h +++ b/src/heap.h @@ -181,7 +181,10 @@ namespace internal { V(Smi, getter_stub_deopt_pc_offset, GetterStubDeoptPCOffset) \ V(Smi, setter_stub_deopt_pc_offset, SetterStubDeoptPCOffset) \ V(JSObject, observation_state, ObservationState) \ - V(Map, external_map, ExternalMap) + V(Map, external_map, ExternalMap) \ + V(Symbol, frozen_symbol, FrozenSymbol) \ + V(SeededNumberDictionary, empty_slow_element_dictionary, \ + EmptySlowElementDictionary) #define ROOT_LIST(V) \ STRONG_ROOT_LIST(V) \ diff --git a/src/objects-inl.h b/src/objects-inl.h index 95a0eca..e974a0e 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -3592,6 +3592,16 @@ bool Map::is_deprecated() { } +void Map::freeze() { + set_bit_field3(IsFrozen::update(bit_field3(), true)); +} + + +bool Map::is_frozen() { + return IsFrozen::decode(bit_field3()); +} + + bool Map::CanBeDeprecated() { int descriptor = LastAdded(); for (int i = 0; i <= descriptor; i++) { diff --git a/src/objects.cc b/src/objects.cc index 527028a..375034c 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -4507,6 +4507,42 @@ MaybeObject* JSObject::TransformToFastProperties(int unused_property_fields) { } +static MUST_USE_RESULT MaybeObject* CopyFastElementsToDictionary( + Isolate* isolate, + FixedArrayBase* array, + int length, + SeededNumberDictionary* dictionary) { + Heap* heap = isolate->heap(); + bool has_double_elements = array->IsFixedDoubleArray(); + for (int i = 0; i < length; i++) { + Object* value = NULL; + if (has_double_elements) { + FixedDoubleArray* double_array = FixedDoubleArray::cast(array); + if (double_array->is_the_hole(i)) { + value = isolate->heap()->the_hole_value(); + } else { + // Objects must be allocated in the old object space, since the + // overall number of HeapNumbers needed for the conversion might + // exceed the capacity of new space, and we would fail repeatedly + // trying to convert the FixedDoubleArray. + MaybeObject* maybe_value_object = + heap->AllocateHeapNumber(double_array->get_scalar(i), TENURED); + if (!maybe_value_object->ToObject(&value)) return maybe_value_object; + } + } else { + value = FixedArray::cast(array)->get(i); + } + if (!value->IsTheHole()) { + PropertyDetails details = PropertyDetails(NONE, NORMAL, 0); + MaybeObject* maybe_result = + dictionary->AddNumberEntry(i, value, details); + if (!maybe_result->To(&dictionary)) return maybe_result; + } + } + return dictionary; +} + + Handle JSObject::NormalizeElements( Handle object) { CALL_HEAP_FUNCTION(object->GetIsolate(), @@ -4538,44 +4574,14 @@ MaybeObject* JSObject::NormalizeElements() { int old_capacity = 0; int used_elements = 0; GetElementsCapacityAndUsage(&old_capacity, &used_elements); - SeededNumberDictionary* dictionary = NULL; - { Object* object; - MaybeObject* maybe = - SeededNumberDictionary::Allocate(GetHeap(), used_elements); - if (!maybe->ToObject(&object)) return maybe; - dictionary = SeededNumberDictionary::cast(object); - } + SeededNumberDictionary* dictionary; + MaybeObject* maybe_dictionary = + SeededNumberDictionary::Allocate(GetHeap(), used_elements); + if (!maybe_dictionary->To(&dictionary)) return maybe_dictionary; - // Copy the elements to the new backing store. - bool has_double_elements = array->IsFixedDoubleArray(); - for (int i = 0; i < length; i++) { - Object* value = NULL; - if (has_double_elements) { - FixedDoubleArray* double_array = FixedDoubleArray::cast(array); - if (double_array->is_the_hole(i)) { - value = GetIsolate()->heap()->the_hole_value(); - } else { - // Objects must be allocated in the old object space, since the - // overall number of HeapNumbers needed for the conversion might - // exceed the capacity of new space, and we would fail repeatedly - // trying to convert the FixedDoubleArray. - MaybeObject* maybe_value_object = - GetHeap()->AllocateHeapNumber(double_array->get_scalar(i), TENURED); - if (!maybe_value_object->ToObject(&value)) return maybe_value_object; - } - } else { - ASSERT(old_map->has_fast_smi_or_object_elements()); - value = FixedArray::cast(array)->get(i); - } - PropertyDetails details = PropertyDetails(NONE, NORMAL, 0); - if (!value->IsTheHole()) { - Object* result; - MaybeObject* maybe_result = - dictionary->AddNumberEntry(i, value, details); - if (!maybe_result->ToObject(&result)) return maybe_result; - dictionary = SeededNumberDictionary::cast(result); - } - } + maybe_dictionary = CopyFastElementsToDictionary( + GetIsolate(), array, length, dictionary); + if (!maybe_dictionary->To(&dictionary)) return maybe_dictionary; // Switch to using the dictionary as the backing storage for elements. if (is_arguments) { @@ -4583,11 +4589,11 @@ MaybeObject* JSObject::NormalizeElements() { } else { // Set the new map first to satify the elements type assert in // set_elements(). - Object* new_map; + Map* new_map; MaybeObject* maybe = GetElementsTransitionMap(GetIsolate(), DICTIONARY_ELEMENTS); - if (!maybe->ToObject(&new_map)) return maybe; - set_map(Map::cast(new_map)); + if (!maybe->To(&new_map)) return maybe; + set_map(new_map); set_elements(dictionary); } @@ -5331,6 +5337,7 @@ MaybeObject* JSObject::PreventExtensions() { // Do a map transition, other objects with this map may still // be extensible. + // TODO(adamk): Extend the NormalizedMapCache to handle non-extensible maps. Map* new_map; MaybeObject* maybe = map()->Copy(); if (!maybe->To(&new_map)) return maybe; @@ -5342,6 +5349,134 @@ MaybeObject* JSObject::PreventExtensions() { } +template +static void FreezeDictionary(Dictionary* dictionary) { + int capacity = dictionary->Capacity(); + for (int i = 0; i < capacity; i++) { + Object* k = dictionary->KeyAt(i); + if (dictionary->IsKey(k)) { + PropertyDetails details = dictionary->DetailsAt(i); + details = details.CopyAddAttributes(FROZEN); + dictionary->DetailsAtPut(i, details); + } + } +} + + +MUST_USE_RESULT MaybeObject* JSObject::Freeze(Isolate* isolate) { + // Freezing non-strict arguments should be handled elsewhere. + ASSERT(!HasNonStrictArgumentsElements()); + + Heap* heap = isolate->heap(); + + if (map()->is_frozen()) return this; + + if (IsAccessCheckNeeded() && + !isolate->MayNamedAccess(this, + heap->undefined_value(), + v8::ACCESS_KEYS)) { + isolate->ReportFailedAccessCheck(this, v8::ACCESS_KEYS); + return heap->false_value(); + } + + if (IsJSGlobalProxy()) { + Object* proto = GetPrototype(); + if (proto->IsNull()) return this; + ASSERT(proto->IsJSGlobalObject()); + return JSObject::cast(proto)->Freeze(isolate); + } + + // It's not possible to freeze objects with external array elements + if (HasExternalArrayElements()) { + HandleScope scope(isolate); + Handle object(this, isolate); + Handle error = + isolate->factory()->NewTypeError( + "cant_prevent_ext_external_array_elements", + HandleVector(&object, 1)); + return isolate->Throw(*error); + } + + SeededNumberDictionary* new_element_dictionary = NULL; + if (!elements()->IsDictionary()) { + int length = IsJSArray() + ? Smi::cast(JSArray::cast(this)->length())->value() + : elements()->length(); + if (length > 0) { + int capacity = 0; + int used = 0; + GetElementsCapacityAndUsage(&capacity, &used); + MaybeObject* maybe_dict = SeededNumberDictionary::Allocate(heap, used); + if (!maybe_dict->To(&new_element_dictionary)) return maybe_dict; + // Make sure that we never go back to fast case. + new_element_dictionary->set_requires_slow_elements(); + + // Move elements to a dictionary; avoid calling NormalizeElements to avoid + // unnecessary transitions. + maybe_dict = CopyFastElementsToDictionary(isolate, elements(), length, + new_element_dictionary); + if (!maybe_dict->To(&new_element_dictionary)) return maybe_dict; + + // Freeze all the elements in the dictionary. + FreezeDictionary(new_element_dictionary); + } else { + // No existing elements, use a pre-allocated empty backing store + new_element_dictionary = heap->empty_slow_element_dictionary(); + } + } + + LookupResult result(isolate); + map()->LookupTransition(this, heap->frozen_symbol(), &result); + if (result.IsTransition()) { + Map* transition_map = result.GetTransitionTarget(); + ASSERT(transition_map->has_dictionary_elements()); + ASSERT(transition_map->is_frozen()); + ASSERT(!transition_map->is_extensible()); + set_map(transition_map); + } else if (HasFastProperties() && map()->CanHaveMoreTransitions()) { + // Create a new descriptor array with fully-frozen properties + int num_descriptors = map()->NumberOfOwnDescriptors(); + DescriptorArray* new_descriptors; + MaybeObject* maybe_descriptors = + map()->instance_descriptors()->CopyUpToAddAttributes(num_descriptors, + FROZEN); + if (!maybe_descriptors->To(&new_descriptors)) return maybe_descriptors; + + Map* new_map; + MaybeObject* maybe_new_map = map()->CopyReplaceDescriptors( + new_descriptors, INSERT_TRANSITION, heap->frozen_symbol()); + if (!maybe_new_map->To(&new_map)) return maybe_new_map; + new_map->freeze(); + new_map->set_is_extensible(false); + new_map->set_elements_kind(DICTIONARY_ELEMENTS); + set_map(new_map); + } else { + // Slow path: need to normalize properties for safety + MaybeObject* maybe = NormalizeProperties(CLEAR_INOBJECT_PROPERTIES, 0); + if (maybe->IsFailure()) return maybe; + + // Create a new map, since other objects with this map may be extensible. + // TODO(adamk): Extend the NormalizedMapCache to handle non-extensible maps. + Map* new_map; + MaybeObject* maybe_copy = map()->Copy(); + if (!maybe_copy->To(&new_map)) return maybe_copy; + new_map->freeze(); + new_map->set_is_extensible(false); + new_map->set_elements_kind(DICTIONARY_ELEMENTS); + set_map(new_map); + + // Freeze dictionary-mode properties + FreezeDictionary(property_dictionary()); + } + + ASSERT(map()->has_dictionary_elements()); + if (new_element_dictionary != NULL) + set_elements(new_element_dictionary); + + return this; +} + + MUST_USE_RESULT MaybeObject* JSObject::DeepCopy(Isolate* isolate) { StackLimitCheck check(isolate); if (check.HasOverflowed()) return isolate->StackOverflow(); @@ -6366,9 +6501,9 @@ MaybeObject* Map::ShareDescriptor(DescriptorArray* descriptors, MaybeObject* Map::CopyReplaceDescriptors(DescriptorArray* descriptors, - Name* name, TransitionFlag flag, - int descriptor_index) { + Name* name, + SimpleTransitionFlag simple_flag) { ASSERT(descriptors->IsSortedNoDuplicates()); Map* result; @@ -6379,14 +6514,8 @@ MaybeObject* Map::CopyReplaceDescriptors(DescriptorArray* descriptors, if (flag == INSERT_TRANSITION && CanHaveMoreTransitions()) { TransitionArray* transitions; - SimpleTransitionFlag simple_flag = - (descriptor_index == descriptors->number_of_descriptors() - 1) - ? SIMPLE_TRANSITION - : FULL_TRANSITION; - ASSERT(name == descriptors->GetKey(descriptor_index)); MaybeObject* maybe_transitions = AddTransition(name, result, simple_flag); if (!maybe_transitions->To(&transitions)) return maybe_transitions; - set_transitions(transitions); result->SetBackPointer(this); } else if (flag != OMIT_TRANSITION_KEEP_REPRESENTATIONS) { @@ -6502,7 +6631,7 @@ MaybeObject* Map::CopyWithPreallocatedFieldDescriptors() { descriptors->CopyUpTo(number_of_own_descriptors); if (!maybe_descriptors->To(&new_descriptors)) return maybe_descriptors; - return CopyReplaceDescriptors(new_descriptors, NULL, OMIT_TRANSITION, 0); + return CopyReplaceDescriptors(new_descriptors, OMIT_TRANSITION); } @@ -6514,7 +6643,7 @@ MaybeObject* Map::Copy() { descriptors->CopyUpTo(number_of_own_descriptors); if (!maybe_descriptors->To(&new_descriptors)) return maybe_descriptors; - return CopyReplaceDescriptors(new_descriptors, NULL, OMIT_TRANSITION, 0); + return CopyReplaceDescriptors(new_descriptors, OMIT_TRANSITION); } @@ -6555,9 +6684,7 @@ MaybeObject* Map::CopyAddDescriptor(Descriptor* descriptor, } Name* key = descriptor->GetKey(); - int insertion_index = new_descriptors->number_of_descriptors() - 1; - - return CopyReplaceDescriptors(new_descriptors, key, flag, insertion_index); + return CopyReplaceDescriptors(new_descriptors, flag, key, SIMPLE_TRANSITION); } @@ -6578,7 +6705,8 @@ MaybeObject* Map::CopyInsertDescriptor(Descriptor* descriptor, } -MaybeObject* DescriptorArray::CopyUpTo(int enumeration_index) { +MaybeObject* DescriptorArray::CopyUpToAddAttributes( + int enumeration_index, PropertyAttributes attributes) { if (enumeration_index == 0) return GetHeap()->empty_descriptor_array(); int size = enumeration_index; @@ -6588,8 +6716,16 @@ MaybeObject* DescriptorArray::CopyUpTo(int enumeration_index) { if (!maybe_descriptors->To(&descriptors)) return maybe_descriptors; DescriptorArray::WhitenessWitness witness(descriptors); - for (int i = 0; i < size; ++i) { - descriptors->CopyFrom(i, this, i, witness); + if (attributes != NONE) { + for (int i = 0; i < size; ++i) { + PropertyDetails details = GetDetails(i).CopyAddAttributes(attributes); + Descriptor desc(GetKey(i), GetValue(i), details); + descriptors->Set(i, &desc, witness); + } + } else { + for (int i = 0; i < size; ++i) { + descriptors->CopyFrom(i, this, i, witness); + } } if (number_of_descriptors() != enumeration_index) descriptors->Sort(); @@ -6630,7 +6766,11 @@ MaybeObject* Map::CopyReplaceDescriptor(DescriptorArray* descriptors, // Re-sort if descriptors were removed. if (new_size != descriptors->length()) new_descriptors->Sort(); - return CopyReplaceDescriptors(new_descriptors, key, flag, insertion_index); + SimpleTransitionFlag simple_flag = + (insertion_index == descriptors->number_of_descriptors() - 1) + ? SIMPLE_TRANSITION + : FULL_TRANSITION; + return CopyReplaceDescriptors(new_descriptors, flag, key, simple_flag); } @@ -13383,13 +13523,13 @@ template class Dictionary; template class Dictionary; template MaybeObject* Dictionary:: - Allocate(Heap* heap, int at_least_space_for); + Allocate(Heap* heap, int at_least_space_for, PretenureFlag pretenure); template MaybeObject* Dictionary:: - Allocate(Heap* heap, int at_least_space_for); + Allocate(Heap* heap, int at_least_space_for, PretenureFlag pretenure); template MaybeObject* Dictionary:: - Allocate(Heap* heap, int n); + Allocate(Heap* heap, int n, PretenureFlag pretenure); template MaybeObject* Dictionary::AtPut( uint32_t, Object*); @@ -14333,10 +14473,15 @@ MaybeObject* MapCache::Put(FixedArray* array, Map* value) { template MaybeObject* Dictionary::Allocate(Heap* heap, - int at_least_space_for) { + int at_least_space_for, + PretenureFlag pretenure) { Object* obj; { MaybeObject* maybe_obj = - HashTable::Allocate(heap, at_least_space_for); + HashTable::Allocate( + heap, + at_least_space_for, + HashTable::USE_DEFAULT_MINIMUM_CAPACITY, + pretenure); if (!maybe_obj->ToObject(&obj)) return maybe_obj; } // Initialize the next enumeration index. diff --git a/src/objects.h b/src/objects.h index 441c6ef..d27af57 100644 --- a/src/objects.h +++ b/src/objects.h @@ -2297,6 +2297,9 @@ class JSObject: public JSReceiver { static Handle PreventExtensions(Handle object); MUST_USE_RESULT MaybeObject* PreventExtensions(); + // ES5 Object.freeze + MUST_USE_RESULT MaybeObject* Freeze(Isolate* isolate); + // Copy object MUST_USE_RESULT MaybeObject* DeepCopy(Isolate* isolate); @@ -2834,7 +2837,13 @@ class DescriptorArray: public FixedArray { int new_size, DescriptorArray* other); - MUST_USE_RESULT MaybeObject* CopyUpTo(int enumeration_index); + MUST_USE_RESULT MaybeObject* CopyUpTo(int enumeration_index) { + return CopyUpToAddAttributes(enumeration_index, NONE); + } + + MUST_USE_RESULT MaybeObject* CopyUpToAddAttributes( + int enumeration_index, + PropertyAttributes attributes); // Sort the instance descriptors by the hash codes of their keys. void Sort(); @@ -3369,8 +3378,10 @@ class Dictionary: public HashTable { } // Returns a new array for dictionary usage. Might return Failure. - MUST_USE_RESULT static MaybeObject* Allocate(Heap* heap, - int at_least_space_for); + MUST_USE_RESULT static MaybeObject* Allocate( + Heap* heap, + int at_least_space_for, + PretenureFlag pretenure = NOT_TENURED); // Ensure enough space for n additional elements. MUST_USE_RESULT MaybeObject* EnsureCapacity(int n, Key key); @@ -5071,6 +5082,7 @@ class Map: public HeapObject { class OwnsDescriptors: public BitField {}; class IsObserved: public BitField {}; class Deprecated: public BitField {}; + class IsFrozen: public BitField {}; // Tells whether the object in the prototype property will be used // for instances created from this function. If the prototype @@ -5373,6 +5385,8 @@ class Map: public HeapObject { inline void set_owns_descriptors(bool is_shared); inline bool is_observed(); inline void set_is_observed(bool is_observed); + inline void freeze(); + inline bool is_frozen(); inline void deprecate(); inline bool is_deprecated(); inline bool CanBeDeprecated(); @@ -5387,9 +5401,9 @@ class Map: public HeapObject { MUST_USE_RESULT MaybeObject* CopyDropDescriptors(); MUST_USE_RESULT MaybeObject* CopyReplaceDescriptors( DescriptorArray* descriptors, - Name* name, TransitionFlag flag, - int descriptor_index); + Name* name = NULL, + SimpleTransitionFlag simple_flag = FULL_TRANSITION); MUST_USE_RESULT MaybeObject* CopyInstallDescriptors( int new_descriptor, DescriptorArray* descriptors); diff --git a/src/property-details.h b/src/property-details.h index 674fc88..46ca4f2 100644 --- a/src/property-details.h +++ b/src/property-details.h @@ -39,7 +39,7 @@ enum PropertyAttributes { DONT_ENUM = v8::DontEnum, DONT_DELETE = v8::DontDelete, - SEALED = DONT_ENUM | DONT_DELETE, + SEALED = DONT_DELETE, FROZEN = SEALED | READ_ONLY, SYMBOLIC = 8, // Used to filter symbol names @@ -180,6 +180,11 @@ class PropertyDetails BASE_EMBEDDED { PropertyDetails CopyWithRepresentation(Representation representation) { return PropertyDetails(value_, representation); } + PropertyDetails CopyAddAttributes(PropertyAttributes new_attributes) { + new_attributes = + static_cast(attributes() | new_attributes); + return PropertyDetails(value_, new_attributes); + } // Conversion for storing details as Object*. explicit inline PropertyDetails(Smi* smi); @@ -237,6 +242,9 @@ class PropertyDetails BASE_EMBEDDED { value_ = RepresentationField::update( value, EncodeRepresentation(representation)); } + PropertyDetails(int value, PropertyAttributes attributes) { + value_ = AttributesField::update(value, attributes); + } uint32_t value_; }; diff --git a/src/runtime.cc b/src/runtime.cc index 61b3549..64573df 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -2692,6 +2692,14 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_ThrowGeneratorStateError) { } +RUNTIME_FUNCTION(MaybeObject*, Runtime_ObjectFreeze) { + NoHandleAllocation ha(isolate); + ASSERT(args.length() == 1); + CONVERT_ARG_CHECKED(JSObject, object, 0); + return object->Freeze(isolate); +} + + MUST_USE_RESULT static MaybeObject* CharFromCode(Isolate* isolate, Object* char_code) { if (char_code->IsNumber()) { @@ -13305,6 +13313,7 @@ ELEMENTS_KIND_CHECK_RUNTIME_FUNCTION(FastSmiOrObjectElements) ELEMENTS_KIND_CHECK_RUNTIME_FUNCTION(FastDoubleElements) ELEMENTS_KIND_CHECK_RUNTIME_FUNCTION(FastHoleyElements) ELEMENTS_KIND_CHECK_RUNTIME_FUNCTION(DictionaryElements) +ELEMENTS_KIND_CHECK_RUNTIME_FUNCTION(NonStrictArgumentsElements) ELEMENTS_KIND_CHECK_RUNTIME_FUNCTION(ExternalPixelElements) ELEMENTS_KIND_CHECK_RUNTIME_FUNCTION(ExternalArrayElements) ELEMENTS_KIND_CHECK_RUNTIME_FUNCTION(ExternalByteElements) diff --git a/src/runtime.h b/src/runtime.h index a37c851..b2f2e96 100644 --- a/src/runtime.h +++ b/src/runtime.h @@ -303,6 +303,9 @@ namespace internal { F(ResumeJSGeneratorObject, 3, 1) \ F(ThrowGeneratorStateError, 1, 1) \ \ + /* ES5 */ \ + F(ObjectFreeze, 1, 1) \ + \ /* Harmony modules */ \ F(IsJSModule, 1, 1) \ \ @@ -423,6 +426,7 @@ namespace internal { F(HasFastDoubleElements, 1, 1) \ F(HasFastHoleyElements, 1, 1) \ F(HasDictionaryElements, 1, 1) \ + F(HasNonStrictArgumentsElements, 1, 1) \ F(HasExternalPixelElements, 1, 1) \ F(HasExternalArrayElements, 1, 1) \ F(HasExternalByteElements, 1, 1) \ diff --git a/src/v8natives.js b/src/v8natives.js index b2ea749..f5ab122 100644 --- a/src/v8natives.js +++ b/src/v8natives.js @@ -1225,20 +1225,27 @@ function ObjectFreeze(obj) { if (!IS_SPEC_OBJECT(obj)) { throw MakeTypeError("called_on_non_object", ["Object.freeze"]); } - if (%IsJSProxy(obj)) { - ProxyFix(obj); - } - var names = ObjectGetOwnPropertyNames(obj); - for (var i = 0; i < names.length; i++) { - var name = names[i]; - var desc = GetOwnProperty(obj, name); - if (desc.isWritable() || desc.isConfigurable()) { - if (IsDataDescriptor(desc)) desc.setWritable(false); - desc.setConfigurable(false); - DefineOwnProperty(obj, name, desc, true); + var isProxy = %IsJSProxy(obj); + if (isProxy || %HasNonStrictArgumentsElements(obj)) { + if (isProxy) { + ProxyFix(obj); } + var names = ObjectGetOwnPropertyNames(obj); + for (var i = 0; i < names.length; i++) { + var name = names[i]; + var desc = GetOwnProperty(obj, name); + if (desc.isWritable() || desc.isConfigurable()) { + if (IsDataDescriptor(desc)) desc.setWritable(false); + desc.setConfigurable(false); + DefineOwnProperty(obj, name, desc, true); + } + } + %PreventExtensions(obj); + } else { + // TODO(adamk): Is it worth going to this fast path if the + // object's properties are already in dictionary mode? + %ObjectFreeze(obj); } - %PreventExtensions(obj); return obj; } diff --git a/test/mjsunit/object-freeze.js b/test/mjsunit/object-freeze.js index c3a9278..6a177a4 100644 --- a/test/mjsunit/object-freeze.js +++ b/test/mjsunit/object-freeze.js @@ -28,6 +28,7 @@ // Tests the Object.freeze and Object.isFrozen methods - ES 15.2.3.9 and // ES 15.2.3.12 +// Flags: --allow-natives-syntax // Test that we throw an error if an object is not passed as argument. var non_objects = new Array(undefined, null, 1, -1, 0, 42.43); @@ -191,3 +192,64 @@ assertFalse(Object.isFrozen(obj5)); // Make sure that Object.freeze returns the frozen object. var obj6 = {} assertTrue(obj6 === Object.freeze(obj6)) + +// Test that the enumerable attribute is unperturbed by freezing. +obj = { x: 42, y: 'foo' }; +Object.defineProperty(obj, 'y', {enumerable: false}); +Object.freeze(obj); +desc = Object.getOwnPropertyDescriptor(obj, 'x'); +assertTrue(desc.enumerable); +desc = Object.getOwnPropertyDescriptor(obj, 'y'); +assertFalse(desc.enumerable); + +// Fast properties should remain fast +obj = { x: 42, y: 'foo' }; +assertTrue(%HasFastProperties(obj)); +Object.freeze(obj); +assertTrue(%HasFastProperties(obj)); + +// Frozen objects should share maps where possible +obj = { prop1: 1, prop2: 2 }; +obj2 = { prop1: 3, prop2: 4 }; +assertTrue(%HaveSameMap(obj, obj2)); +Object.freeze(obj); +Object.freeze(obj2); +assertTrue(%HaveSameMap(obj, obj2)); + +// Frozen objects should share maps even when they have elements +obj = { prop1: 1, prop2: 2, 75: 'foo' }; +obj2 = { prop1: 3, prop2: 4, 150: 'bar' }; +assertTrue(%HaveSameMap(obj, obj2)); +Object.freeze(obj); +Object.freeze(obj2); +assertTrue(%HaveSameMap(obj, obj2)); + +// Setting elements after freezing should not be allowed +obj = { prop: 'thing' }; +Object.freeze(obj); +obj[0] = 'hello'; +assertFalse(obj.hasOwnProperty(0)); + +// Freezing an object in dictionary mode should work +obj = { }; +for (var i = 0; i < 100; ++i) { + obj['x' + i] = i; +} +assertFalse(%HasFastProperties(obj)); +Object.freeze(obj); +assertFalse(%HasFastProperties(obj)); +assertTrue(Object.isFrozen(obj)); +assertFalse(Object.isExtensible(obj)); +for (var i = 0; i < 100; ++i) { + desc = Object.getOwnPropertyDescriptor(obj, 'x' + i); + assertFalse(desc.writable); + assertFalse(desc.configurable); +} + +// Freezing arguments should work +var func = function(arg) { + Object.freeze(arguments); + assertTrue(Object.isFrozen(arguments)); +}; +func('hello', 'world'); +func('goodbye', 'world'); -- 2.7.4