diff --git a/src/elements.cc b/src/elements.cc index 8323c8950f..6b20c134d0 100644 --- a/src/elements.cc +++ b/src/elements.cc @@ -518,7 +518,8 @@ class ElementsAccessorBase : public ElementsAccessor { uint32_t end) { if (IsFastPackedElementsKind(kind())) return true; for (uint32_t i = start; i < end; i++) { - if (!ElementsAccessorSubclass::HasElementImpl(holder, i, backing_store)) { + if (!ElementsAccessorSubclass::HasElementImpl(holder, i, backing_store, + NONE)) { return false; } } @@ -544,15 +545,17 @@ class ElementsAccessorBase : public ElementsAccessor { } virtual bool HasElement(Handle holder, uint32_t index, - Handle backing_store) final { + Handle backing_store, + PropertyAttributes filter) final { return ElementsAccessorSubclass::HasElementImpl(holder, index, - backing_store); + backing_store, filter); } static bool HasElementImpl(Handle holder, uint32_t index, - Handle backing_store) { + Handle backing_store, + PropertyAttributes filter) { return ElementsAccessorSubclass::GetEntryForIndexImpl( - *holder, *backing_store, index) != kMaxUInt32; + *holder, *backing_store, index, filter) != kMaxUInt32; } virtual Handle Get(Handle backing_store, @@ -868,25 +871,51 @@ class ElementsAccessorBase : public ElementsAccessor { from, from_start, *to, from_kind, to_start, packed_size, copy_size); } + static void CollectElementIndicesImpl(Handle object, + Handle backing_store, + KeyAccumulator* keys, uint32_t range, + PropertyAttributes filter, + uint32_t offset) { + uint32_t length = 0; + if (object->IsJSArray()) { + length = Smi::cast(JSArray::cast(*object)->length())->value(); + } else { + length = + ElementsAccessorSubclass::GetCapacityImpl(*object, *backing_store); + } + if (range < length) length = range; + for (uint32_t i = offset; i < length; i++) { + if (!ElementsAccessorSubclass::HasElementImpl(object, i, backing_store, + filter)) + continue; + keys->AddKey(i); + } + } + + virtual void CollectElementIndices(Handle object, + Handle backing_store, + KeyAccumulator* keys, uint32_t range, + PropertyAttributes filter, + uint32_t offset) final { + ElementsAccessorSubclass::CollectElementIndicesImpl( + object, backing_store, keys, range, filter, offset); + }; + virtual void AddElementsToKeyAccumulator(Handle receiver, KeyAccumulator* accumulator, - KeyFilter filter) final { + AddKeyConversion convert) final { Handle from(receiver->elements()); uint32_t add_length = ElementsAccessorSubclass::GetCapacityImpl(*receiver, *from); if (add_length == 0) return; - accumulator->PrepareForComparisons(add_length); - int prev_key_count = accumulator->GetLength(); + for (uint32_t i = 0; i < add_length; i++) { if (!ElementsAccessorSubclass::HasEntryImpl(*from, i)) continue; Handle value = ElementsAccessorSubclass::GetImpl(from, i); DCHECK(!value->IsTheHole()); DCHECK(!value->IsAccessorPair()); DCHECK(!value->IsExecutableAccessorInfo()); - if (filter == SKIP_SYMBOLS && value->IsSymbol()) { - continue; - } - accumulator->AddKey(value, prev_key_count); + accumulator->AddKey(value, convert); } } @@ -895,7 +924,8 @@ class ElementsAccessorBase : public ElementsAccessor { return backing_store->length(); } - uint32_t GetCapacity(JSObject* holder, FixedArrayBase* backing_store) final { + virtual uint32_t GetCapacity(JSObject* holder, + FixedArrayBase* backing_store) final { return ElementsAccessorSubclass::GetCapacityImpl(holder, backing_store); } @@ -910,7 +940,8 @@ class ElementsAccessorBase : public ElementsAccessor { static uint32_t GetEntryForIndexImpl(JSObject* holder, FixedArrayBase* backing_store, - uint32_t index) { + uint32_t index, + PropertyAttributes filter) { if (IsHoleyElementsKind(kind())) { return index < ElementsAccessorSubclass::GetCapacityImpl(holder, backing_store) && @@ -928,7 +959,7 @@ class ElementsAccessorBase : public ElementsAccessor { FixedArrayBase* backing_store, uint32_t index) final { return ElementsAccessorSubclass::GetEntryForIndexImpl(holder, backing_store, - index); + index, NONE); } static PropertyDetails GetDetailsImpl(FixedArrayBase* backing_store, @@ -1090,19 +1121,50 @@ class DictionaryElementsAccessor } static uint32_t GetEntryForIndexImpl(JSObject* holder, FixedArrayBase* store, - uint32_t index) { + uint32_t index, + PropertyAttributes filter) { DisallowHeapAllocation no_gc; - SeededNumberDictionary* dict = SeededNumberDictionary::cast(store); - int entry = dict->FindEntry(index); - return entry == SeededNumberDictionary::kNotFound - ? kMaxUInt32 - : static_cast(entry); + SeededNumberDictionary* dictionary = SeededNumberDictionary::cast(store); + int entry = dictionary->FindEntry(index); + if (entry == SeededNumberDictionary::kNotFound) return kMaxUInt32; + if (filter != NONE) { + PropertyDetails details = dictionary->DetailsAt(entry); + PropertyAttributes attr = details.attributes(); + if ((attr & filter) != 0) return kMaxUInt32; + } + return static_cast(entry); } static PropertyDetails GetDetailsImpl(FixedArrayBase* backing_store, uint32_t entry) { return SeededNumberDictionary::cast(backing_store)->DetailsAt(entry); } + + static void CollectElementIndicesImpl(Handle object, + Handle backing_store, + KeyAccumulator* keys, uint32_t range, + PropertyAttributes filter, + uint32_t offset) { + Handle dictionary = + Handle::cast(backing_store); + int capacity = dictionary->Capacity(); + for (int i = 0; i < capacity; i++) { + Object* k = dictionary->KeyAt(i); + if (!dictionary->IsKey(k)) continue; + if (k->FilterKey(filter)) continue; + if (dictionary->IsDeleted(i)) continue; + DCHECK(k->IsNumber()); + DCHECK_LE(k->Number(), kMaxUInt32); + uint32_t index = static_cast(k->Number()); + if (index < offset) continue; + PropertyDetails details = dictionary->DetailsAt(i); + PropertyAttributes attr = details.attributes(); + if ((attr & filter) != 0) continue; + keys->AddKey(index); + } + + keys->SortCurrentElementsList(); + } }; @@ -1759,7 +1821,8 @@ class TypedElementsAccessor static uint32_t GetEntryForIndexImpl(JSObject* holder, FixedArrayBase* backing_store, - uint32_t index) { + uint32_t index, + PropertyAttributes filter) { return index < AccessorClass::GetCapacityImpl(holder, backing_store) ? index : kMaxUInt32; @@ -1893,14 +1956,15 @@ class SloppyArgumentsElementsAccessor static uint32_t GetEntryForIndexImpl(JSObject* holder, FixedArrayBase* parameters, - uint32_t index) { + uint32_t index, + PropertyAttributes filter) { FixedArray* parameter_map = FixedArray::cast(parameters); Object* probe = GetParameterMapArg(parameter_map, index); if (!probe->IsTheHole()) return index; FixedArray* arguments = FixedArray::cast(parameter_map->get(1)); - uint32_t entry = - ArgumentsAccessor::GetEntryForIndexImpl(holder, arguments, index); + uint32_t entry = ArgumentsAccessor::GetEntryForIndexImpl(holder, arguments, + index, filter); if (entry == kMaxUInt32) return entry; return (parameter_map->length() - 2) + entry; } diff --git a/src/elements.h b/src/elements.h index c238a7a217..9530d4bdec 100644 --- a/src/elements.h +++ b/src/elements.h @@ -22,6 +22,14 @@ class ElementsAccessor { const char* name() const { return name_; } + // Returns a shared ElementsAccessor for the specified ElementsKind. + static ElementsAccessor* ForKind(ElementsKind elements_kind) { + DCHECK(static_cast(elements_kind) < kElementsKindCount); + return elements_accessors_[elements_kind]; + } + + static ElementsAccessor* ForArray(Handle array); + // Checks the elements of an object for consistency, asserting when a problem // is found. virtual void Validate(Handle obj) = 0; @@ -30,12 +38,19 @@ class ElementsAccessor { // without iterating up the prototype chain. The caller can optionally pass // in the backing store to use for the check, which must be compatible with // the ElementsKind of the ElementsAccessor. If backing_store is NULL, the - // holder->elements() is used as the backing store. + // holder->elements() is used as the backing store. If a |filter| is + // specified the PropertyAttributes of the element at the given index + // are compared to the given |filter|. If they match/overlap the given + // index is ignored. Note that only Dictionary elements have custom + // PropertyAttributes associated, hence the |filter| argument is ignored for + // all but DICTIONARY_ELEMENTS and SLOW_SLOPPY_ARGUMENTS_ELEMENTS. virtual bool HasElement(Handle holder, uint32_t index, - Handle backing_store) = 0; + Handle backing_store, + PropertyAttributes filter = NONE) = 0; - inline bool HasElement(Handle holder, uint32_t index) { - return HasElement(holder, index, handle(holder->elements())); + inline bool HasElement(Handle holder, uint32_t index, + PropertyAttributes filter = NONE) { + return HasElement(holder, index, handle(holder->elements()), filter); } // Returns true if the backing store is compact in the given range @@ -97,20 +112,31 @@ class ElementsAccessor { *from_holder, 0, from_kind, to, 0, kCopyToEndAndInitializeToHole); } - virtual void GrowCapacityAndConvert(Handle object, - uint32_t capacity) = 0; + // Copy all indices that have elements from |object| into the given + // KeyAccumulator. For Dictionary-based element-kinds we filter out elements + // whose PropertyAttribute match |filter|. + virtual void CollectElementIndices(Handle object, + Handle backing_store, + KeyAccumulator* keys, + uint32_t range = kMaxUInt32, + PropertyAttributes filter = NONE, + uint32_t offset = 0) = 0; + + inline void CollectElementIndices(Handle object, + KeyAccumulator* keys, + uint32_t range = kMaxUInt32, + PropertyAttributes filter = NONE, + uint32_t offset = 0) { + CollectElementIndices(object, handle(object->elements()), keys, range, + filter, offset); + } virtual void AddElementsToKeyAccumulator(Handle receiver, KeyAccumulator* accumulator, - KeyFilter filter) = 0; + AddKeyConversion convert) = 0; - // Returns a shared ElementsAccessor for the specified ElementsKind. - static ElementsAccessor* ForKind(ElementsKind elements_kind) { - DCHECK(static_cast(elements_kind) < kElementsKindCount); - return elements_accessors_[elements_kind]; - } - - static ElementsAccessor* ForArray(Handle array); + virtual void GrowCapacityAndConvert(Handle object, + uint32_t capacity) = 0; static void InitializeOncePerProcess(); static void TearDown(); @@ -158,8 +184,6 @@ class ElementsAccessor { static ElementsAccessor* ForArray(FixedArrayBase* array); - virtual uint32_t GetCapacity(JSObject* holder, - FixedArrayBase* backing_store) = 0; // Element handlers distinguish between entries and indices when they // manipulate elements. Entries refer to elements in terms of their location @@ -176,6 +200,8 @@ class ElementsAccessor { uint32_t entry) = 0; private: + virtual uint32_t GetCapacity(JSObject* holder, + FixedArrayBase* backing_store) = 0; static ElementsAccessor** elements_accessors_; const char* name_; diff --git a/src/objects-inl.h b/src/objects-inl.h index f003205800..3bda179712 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -286,6 +286,24 @@ bool Object::KeyEquals(Object* second) { } +bool Object::FilterKey(PropertyAttributes filter) { + if ((filter & SYMBOLIC) && IsSymbol()) { + return true; + } + + if ((filter & PRIVATE_SYMBOL) && IsSymbol() && + Symbol::cast(this)->is_private()) { + return true; + } + + if ((filter & STRING) && !IsSymbol()) { + return true; + } + + return false; +} + + Handle Object::NewStorageFor(Isolate* isolate, Handle object, Representation representation) { diff --git a/src/objects.cc b/src/objects.cc index 3c95e03f29..97b4a0e178 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -24,13 +24,14 @@ #include "src/deoptimizer.h" #include "src/elements.h" #include "src/execution.h" -#include "src/field-index-inl.h" #include "src/field-index.h" +#include "src/field-index-inl.h" #include "src/full-codegen/full-codegen.h" #include "src/hydrogen.h" #include "src/ic/ic.h" #include "src/interpreter/bytecodes.h" #include "src/isolate-inl.h" +#include "src/list.h" #include "src/log.h" #include "src/lookup.h" #include "src/macro-assembler.h" @@ -7329,24 +7330,6 @@ bool JSReceiver::IsSimpleEnum() { } -static bool FilterKey(Object* key, PropertyAttributes filter) { - if ((filter & SYMBOLIC) && key->IsSymbol()) { - return true; - } - - if ((filter & PRIVATE_SYMBOL) && - key->IsSymbol() && Symbol::cast(key)->is_private()) { - return true; - } - - if ((filter & STRING) && !key->IsSymbol()) { - return true; - } - - return false; -} - - int Map::NumberOfDescribedProperties(DescriptorFlag which, PropertyAttributes filter) { int result = 0; @@ -7356,7 +7339,7 @@ int Map::NumberOfDescribedProperties(DescriptorFlag which, : NumberOfOwnDescriptors(); for (int i = 0; i < limit; i++) { if ((descs->GetDetails(i).attributes() & filter) == 0 && - !FilterKey(descs->GetKey(i), filter)) { + !descs->GetKey(i)->FilterKey(filter)) { result++; } } @@ -7507,124 +7490,225 @@ Handle JSObject::GetEnumPropertyKeys(Handle object, } -Handle KeyAccumulator::GetKeys() { +KeyAccumulator::~KeyAccumulator() { + for (int i = 0; i < elements_.length(); i++) { + delete elements_[i]; + } +} + + +Handle KeyAccumulator::GetKeys(GetKeysConversion convert) { if (length_ == 0) { return isolate_->factory()->empty_fixed_array(); } - if (set_.is_null()) { - keys_->Shrink(length_); - return keys_; - } - // copy over results from set_ + // Make sure we have all the lengths collected. + NextPrototype(); + + // Assemble the result array by first adding the element keys and then + // the property keys. We use the total number of keys per level in + // |protoLengths_| and the available element keys in the corresponding bucket + // in |elements_| to deduce the number of keys to take from the |properties_| + // set. Handle result = isolate_->factory()->NewFixedArray(length_); - for (int i = 0; i < length_; i++) { - result->set(i, set_->KeyAt(i)); + int index = 0; + int properties_index = 0; + for (int level = 0; level < levelLengths_.length(); level++) { + int num_total = levelLengths_[level]; + int num_elements = 0; + if (num_total < 0) { + // If the total is negative, the current level contains properties from a + // proxy, hence we skip the integer keys in |elements_| since proxies + // define the complete ordering. + num_total = -num_total; + } else if (level < elements_.length()) { + List* elements = elements_[level]; + num_elements = elements->length(); + for (int i = 0; i < num_elements; i++) { + Handle key; + if (convert == KEEP_NUMBERS) { + key = isolate_->factory()->NewNumberFromUint(elements->at(i)); + } else { + key = isolate_->factory()->Uint32ToString(elements->at(i)); + } + result->set(index, *key); + index++; + } + } + // Add the property keys for this prototype level. + int num_properties = num_total - num_elements; + for (int i = 0; i < num_properties; i++) { + Object* key = properties_->KeyAt(properties_index); + result->set(index, key); + index++; + properties_index++; + } } + DCHECK_EQ(index, length_); return result; } -void KeyAccumulator::AddKey(Handle key, int check_limit) { -#ifdef ENABLE_SLOW_DCHECKS - if (FLAG_enable_slow_asserts) { - DCHECK(key->IsNumber() || key->IsName()); +namespace { + +class FindKey { + public: + explicit FindKey(uint32_t key) : key_(key) {} + int operator()(uint32_t* entry) { + if (*entry == key_) return 0; + return *entry < key_ ? -1 : 1; } -#endif - if (!set_.is_null()) { - set_ = OrderedHashSet::Add(set_, key); - length_ = set_->NumberOfElements(); - return; + + private: + uint32_t key_; +}; + + +bool AccumulatorHasKey(List* sub_elements, uint32_t key) { + int index = SortedListBSearch(*sub_elements, FindKey(key)); + return index != -1; +} + +} // namespace + + +bool KeyAccumulator::AddKey(uint32_t key) { + // Make sure we do not add keys to a proxy-level (see AddKeysFromProxy). + DCHECK_LE(0, levelLength_); + int lookup_limit = elements_.length(); + for (int i = 0; i < lookup_limit; i++) { + if (AccumulatorHasKey(elements_[i], key)) return false; } - // check if we already have the key in the case we are still using - // the keys_ FixedArray - check_limit = Min(check_limit, length_); - for (int i = 0; i < check_limit; i++) { - Object* current = keys_->get(i); - if (current->KeyEquals(*key)) return; - } - EnsureCapacity(length_); - keys_->set(length_, *key); + elements_[lookup_limit - 1]->Add(key); length_++; + levelLength_++; + return true; } -void KeyAccumulator::AddKeys(Handle array, KeyFilter filter) { - int add_length = array->length(); - if (add_length == 0) return; - if (keys_.is_null() && filter == INCLUDE_SYMBOLS) { - keys_ = array; - length_ = keys_->length(); - return; - } - PrepareForComparisons(add_length); - int previous_key_count = length_; - for (int i = 0; i < add_length; i++) { - Handle current(array->get(i), isolate_); - if (filter == SKIP_SYMBOLS && current->IsSymbol()) continue; - AddKey(current, previous_key_count); - } +bool KeyAccumulator::AddKey(Object* key, AddKeyConversion convert) { + return AddKey(handle(key, isolate_), convert); } -void KeyAccumulator::AddKeys(Handle array_like, KeyFilter filter) { - DCHECK(array_like->IsJSArray() || array_like->HasSloppyArgumentsElements()); - ElementsAccessor* accessor = array_like->GetElementsAccessor(); - accessor->AddElementsToKeyAccumulator(array_like, this, filter); -} - - -void KeyAccumulator::PrepareForComparisons(int count) { - // Depending on how many comparisons we do we should switch to the - // hash-table-based checks which have a one-time overhead for - // initializing but O(1) for HasKey checks. - if (!set_.is_null()) return; - // These limits were obtained through evaluation of several microbenchmarks. - if (length_ * count < 100) return; - // Don't use a set for few elements - if (length_ < 100 && count < 20) return; - set_ = OrderedHashSet::Allocate(isolate_, length_); - for (int i = 0; i < length_; i++) { - Handle value(keys_->get(i), isolate_); - set_ = OrderedHashSet::Add(set_, value); +bool KeyAccumulator::AddKey(Handle key, AddKeyConversion convert) { + if (filter_ == SKIP_SYMBOLS && key->IsSymbol()) { + return false; } -} - - -void KeyAccumulator::EnsureCapacity(int capacity) { - if (keys_.is_null() || keys_->length() <= capacity) { - Grow(); - } -} - - -void KeyAccumulator::Grow() { - // The OrderedHashSet handles growing by itself. - if (!set_.is_null()) return; - // Otherwise, grow the internal keys_ FixedArray - int capacity = keys_.is_null() ? 16 : keys_->length() * 2 + 16; - Handle new_keys = isolate_->factory()->NewFixedArray(capacity); - if (keys_.is_null()) { - keys_ = new_keys; - return; - } - int buffer_length = keys_->length(); - { - DisallowHeapAllocation no_gc; - WriteBarrierMode mode = new_keys->GetWriteBarrierMode(no_gc); - for (int i = 0; i < buffer_length; i++) { - new_keys->set(i, keys_->get(i), mode); + // Make sure we do not add keys to a proxy-level (see AddKeysFromProxy). + DCHECK_LE(0, levelLength_); + // In some cases (e.g. proxies) we might get in String-converted ints which + // should be added to the elements list instead of the properties. For + // proxies we have to convert as well but also respect the original order. + // Therefore we add a converted key to both sides + if (convert == CONVERT_TO_ARRAY_INDEX || convert == PROXY_MAGIC) { + uint32_t index = 0; + int prev_length = length_; + int prev_proto = levelLength_; + bool was_array_index = false; + bool key_was_added = false; + if ((key->IsString() && Handle::cast(key)->AsArrayIndex(&index)) || + key->ToArrayIndex(&index)) { + key_was_added = AddKey(index); + was_array_index = true; + if (convert == CONVERT_TO_ARRAY_INDEX) return key_was_added; + } + if (was_array_index && convert == PROXY_MAGIC) { + // If we had an array index (number) and it wasn't added, the key + // already existed before, hence we cannot add it to the properties + // keys as it would lead to duplicate entries. + if (!key_was_added) { + return false; + } + length_ = prev_length; + levelLength_ = prev_proto; } } - keys_ = new_keys; + if (properties_.is_null()) { + properties_ = OrderedHashSet::Allocate(isolate_, 16); + } + // TODO(cbruni): remove this conversion once we throw the correct TypeError + // for non-string/symbol elements returned by proxies + if (convert == PROXY_MAGIC && key->IsNumber()) { + key = isolate_->factory()->NumberToString(key); + } + int prev_size = properties_->NumberOfElements(); + properties_ = OrderedHashSet::Add(properties_, key); + if (prev_size < properties_->NumberOfElements()) { + length_++; + levelLength_++; + } + return true; +} + + +void KeyAccumulator::AddKeys(Handle array, + AddKeyConversion convert) { + int add_length = array->length(); + if (add_length == 0) return; + for (int i = 0; i < add_length; i++) { + Handle current(array->get(i), isolate_); + AddKey(current); + } +} + + +void KeyAccumulator::AddKeys(Handle array_like, + AddKeyConversion convert) { + DCHECK(array_like->IsJSArray() || array_like->HasSloppyArgumentsElements()); + ElementsAccessor* accessor = array_like->GetElementsAccessor(); + accessor->AddElementsToKeyAccumulator(array_like, this, convert); +} + + +void KeyAccumulator::AddKeysFromProxy(Handle array_like) { + // Proxies define a complete list of keys with no distinction of + // elements and properties, which breaks the normal assumption for the + // KeyAccumulator. + AddKeys(array_like, PROXY_MAGIC); + // Invert the current length to indicate a present proxy, so we can ignore + // element keys for this level. Otherwise we would not fully respect the order + // given by the proxy. + levelLength_ = -levelLength_; +} + + +namespace { + +// Used for sorting indices in a List. +int compareUInt32(const uint32_t* ap, const uint32_t* bp) { + uint32_t a = *ap; + uint32_t b = *bp; + return (a == b) ? 0 : (a < b) ? -1 : 1; +} + + +} // namespace + + +void KeyAccumulator::SortCurrentElementsList() { + if (elements_.length() == 0) return; + List* element_keys = elements_[elements_.length() - 1]; + element_keys->Sort(&compareUInt32); +} + + +void KeyAccumulator::NextPrototype() { + // Store the protoLength on the first call of this method. + if (!elements_.is_empty()) { + levelLengths_.Add(levelLength_); + } + elements_.Add(new List()); + levelLength_ = 0; } MaybeHandle JSReceiver::GetKeys(Handle object, KeyCollectionType type, - KeyFilter filter) { + KeyFilter filter, + GetKeysConversion getConversion) { USE(ContainsOnlyValidKeys); Isolate* isolate = object->GetIsolate(); - KeyAccumulator accumulator(isolate); + KeyAccumulator accumulator(isolate, filter); Handle arguments_function( JSFunction::cast(isolate->sloppy_arguments_map()->GetConstructor())); @@ -7635,6 +7719,7 @@ MaybeHandle JSReceiver::GetKeys(Handle object, for (PrototypeIterator iter(isolate, object, PrototypeIterator::START_AT_RECEIVER); !iter.IsAtEnd(end); iter.Advance()) { + accumulator.NextPrototype(); if (PrototypeIterator::GetCurrent(iter)->IsJSProxy()) { Handle proxy = PrototypeIterator::GetCurrent(iter); Handle args[] = { proxy }; @@ -7647,7 +7732,7 @@ MaybeHandle JSReceiver::GetKeys(Handle object, arraysize(args), args), FixedArray); - accumulator.AddKeys(Handle::cast(names), filter); + accumulator.AddKeysFromProxy(Handle::cast(names)); break; } @@ -7663,21 +7748,16 @@ MaybeHandle JSReceiver::GetKeys(Handle object, break; } - // Compute the element keys. - Handle element_keys = - isolate->factory()->NewFixedArray(current->NumberOfEnumElements()); - current->GetEnumElementKeys(*element_keys); - accumulator.AddKeys(element_keys, filter); - DCHECK(ContainsOnlyValidKeys(accumulator.GetKeys())); + JSObject::CollectOwnElementKeys(current, &accumulator, + static_cast(DONT_ENUM)); // Add the element keys from the interceptor. if (current->HasIndexedInterceptor()) { Handle result; - if (JSObject::GetKeysForIndexedInterceptor( - current, object).ToHandle(&result)) { - accumulator.AddKeys(result, filter); + if (JSObject::GetKeysForIndexedInterceptor(current, object) + .ToHandle(&result)) { + accumulator.AddKeys(result, CONVERT_TO_ARRAY_INDEX); } - DCHECK(ContainsOnlyValidKeys(accumulator.GetKeys())); } if (filter == SKIP_SYMBOLS) { @@ -7697,33 +7777,27 @@ MaybeHandle JSReceiver::GetKeys(Handle object, !current->HasNamedInterceptor() && !current->HasIndexedInterceptor()); // Compute the property keys and cache them if possible. - Handle enum_keys = JSObject::GetEnumPropertyKeys(current, cache_enum_length); - accumulator.AddKeys(enum_keys, filter); + accumulator.AddKeys(enum_keys); } else { DCHECK(filter == INCLUDE_SYMBOLS); PropertyAttributes attr_filter = static_cast(DONT_ENUM | PRIVATE_SYMBOL); - Handle property_keys = isolate->factory()->NewFixedArray( - current->NumberOfOwnProperties(attr_filter)); - current->GetOwnPropertyNames(*property_keys, 0, attr_filter); - accumulator.AddKeys(property_keys, filter); + JSObject::CollectOwnElementKeys(current, &accumulator, attr_filter); } - DCHECK(ContainsOnlyValidKeys(accumulator.GetKeys())); // Add the property keys from the interceptor. if (current->HasNamedInterceptor()) { Handle result; - if (JSObject::GetKeysForNamedInterceptor( - current, object).ToHandle(&result)) { - accumulator.AddKeys(result, filter); + if (JSObject::GetKeysForNamedInterceptor(current, object) + .ToHandle(&result)) { + accumulator.AddKeys(result); } - DCHECK(ContainsOnlyValidKeys(accumulator.GetKeys())); } } - Handle keys = accumulator.GetKeys(); + Handle keys = accumulator.GetKeys(getConversion); DCHECK(ContainsOnlyValidKeys(keys)); return keys; } @@ -14680,7 +14754,7 @@ int JSObject::GetOwnPropertyNames(FixedArray* storage, int index, DescriptorArray* descs = map()->instance_descriptors(); for (int i = 0; i < real_size; i++) { if ((descs->GetDetails(i).attributes() & filter) == 0 && - !FilterKey(descs->GetKey(i), filter)) { + !descs->GetKey(i)->FilterKey(filter)) { storage->set(index++, descs->GetKey(i)); } } @@ -14715,12 +14789,35 @@ int JSObject::NumberOfEnumElements() { } +void JSObject::CollectOwnElementKeys(Handle object, + KeyAccumulator* keys, + PropertyAttributes filter) { + uint32_t string_keys = 0; + + // If this is a String wrapper, add the string indices first, + // as they're guaranteed to precede the elements in numerical order + // and ascending order is required by ECMA-262, 6th, 9.1.12. + if (object->IsJSValue()) { + Object* val = JSValue::cast(*object)->value(); + if (val->IsString()) { + String* str = String::cast(val); + string_keys = str->length(); + for (uint32_t i = 0; i < string_keys; i++) { + keys->AddKey(i); + } + } + } + ElementsAccessor* accessor = object->GetElementsAccessor(); + accessor->CollectElementIndices(object, keys, kMaxUInt32, filter, 0); +} + + int JSObject::GetOwnElementKeys(FixedArray* storage, PropertyAttributes filter) { int counter = 0; // If this is a String wrapper, add the string indices first, - // as they're guaranteed to preced the elements in numerical order + // as they're guaranteed to precede the elements in numerical order // and ascending order is required by ECMA-262, 6th, 9.1.12. if (IsJSValue()) { Object* val = JSValue::cast(this)->value(); @@ -14845,11 +14942,6 @@ int JSObject::GetOwnElementKeys(FixedArray* storage, } -int JSObject::GetEnumElementKeys(FixedArray* storage) { - return GetOwnElementKeys(storage, static_cast(DONT_ENUM)); -} - - const char* Symbol::PrivateSymbolToName() const { Heap* heap = GetIsolate()->heap(); #define SYMBOL_CHECK_AND_PRINT(name) \ @@ -16373,7 +16465,7 @@ int Dictionary::NumberOfElementsFilterAttributes( int result = 0; for (int i = 0; i < capacity; i++) { Object* k = this->KeyAt(i); - if (this->IsKey(k) && !FilterKey(k, filter)) { + if (this->IsKey(k) && !k->FilterKey(filter)) { if (this->IsDeleted(i)) continue; PropertyDetails details = this->DetailsAt(i); PropertyAttributes attr = details.attributes(); @@ -16389,7 +16481,7 @@ bool Dictionary::HasComplexElements() { int capacity = this->Capacity(); for (int i = 0; i < capacity; i++) { Object* k = this->KeyAt(i); - if (this->IsKey(k) && !FilterKey(k, NONE)) { + if (this->IsKey(k) && !k->FilterKey(NONE)) { if (this->IsDeleted(i)) continue; PropertyDetails details = this->DetailsAt(i); if (details.type() == ACCESSOR_CONSTANT) return true; @@ -16448,7 +16540,7 @@ int Dictionary::CopyKeysTo( int capacity = this->Capacity(); for (int i = 0; i < capacity; i++) { Object* k = this->KeyAt(i); - if (this->IsKey(k) && !FilterKey(k, filter)) { + if (this->IsKey(k) && !k->FilterKey(filter)) { if (this->IsDeleted(i)) continue; PropertyDetails details = this->DetailsAt(i); PropertyAttributes attr = details.attributes(); diff --git a/src/objects.h b/src/objects.h index 82653c128f..6c209b1792 100644 --- a/src/objects.h +++ b/src/objects.h @@ -854,6 +854,7 @@ class FixedArrayBase; class FunctionLiteral; class GlobalObject; class JSBuiltinsObject; +class KeyAccumulator; class LayoutDescriptor; class LiteralsArray; class LookupIterator; @@ -1089,6 +1090,8 @@ class Object { // 1 all refer to the same property, so this helper will return true. inline bool KeyEquals(Object* other); + inline bool FilterKey(PropertyAttributes filter); + Handle OptimalType(Isolate* isolate, Representation representation); inline static Handle NewStorageFor(Isolate* isolate, @@ -1791,6 +1794,9 @@ enum AccessorComponent { enum KeyFilter { SKIP_SYMBOLS, INCLUDE_SYMBOLS }; +enum GetKeysConversion { KEEP_NUMBERS, CONVERT_TO_STRING }; + + enum ShouldThrow { THROW_ON_ERROR, DONT_THROW }; @@ -1903,7 +1909,8 @@ class JSReceiver: public HeapObject { // "for (n in object) { }". MUST_USE_RESULT static MaybeHandle GetKeys( Handle object, KeyCollectionType type, - KeyFilter filter = SKIP_SYMBOLS); + KeyFilter filter = SKIP_SYMBOLS, + GetKeysConversion getConversion = KEEP_NUMBERS); private: DISALLOW_IMPLICIT_CONSTRUCTORS(JSReceiver); @@ -2233,6 +2240,9 @@ class JSObject: public JSReceiver { // Returns the number of elements on this object filtering out elements // with the specified attributes (ignoring interceptors). int GetOwnElementKeys(FixedArray* storage, PropertyAttributes filter); + static void CollectOwnElementKeys(Handle object, + KeyAccumulator* keys, + PropertyAttributes filter); // Count and fill in the enumerable elements into storage. // (storage->length() == NumberOfEnumElements()). // If storage is NULL, will count the elements without adding @@ -10714,26 +10724,62 @@ class BooleanBit : public AllStatic { }; +enum AddKeyConversion { DO_NOT_CONVERT, CONVERT_TO_ARRAY_INDEX, PROXY_MAGIC }; + +// This is a helper class for JSReceiver::GetKeys which collects and sorts keys. +// GetKeys needs to sort keys per prototype level, first showing the integer +// indices from elements then the strings from the properties. However, this +// does not apply to proxies which are in full control of how the keys are +// sorted. +// +// For performance reasons the KeyAccumulator internally separates integer +// keys in |elements_| into sorted lists per prototype level. String keys are +// collected in |properties_|, a single OrderedHashSet. To separate the keys per +// level later when assembling the final list, |levelLengths_| keeps track of +// the total number of keys (integers + strings) per level. +// +// Only unique keys are kept by the KeyAccumulator, strings are stored in a +// HashSet for inexpensive lookups. Integer keys are kept in sorted lists which +// are more compact and allow for reasonably fast includes check. class KeyAccumulator final BASE_EMBEDDED { public: - explicit KeyAccumulator(Isolate* isolate) : isolate_(isolate), length_(0) {} + explicit KeyAccumulator(Isolate* isolate, + KeyFilter filter = KeyFilter::SKIP_SYMBOLS) + : isolate_(isolate), filter_(filter), length_(0), levelLength_(0) {} + ~KeyAccumulator(); - void AddKey(Handle key, int check_limit); - void AddKeys(Handle array, KeyFilter filter); - void AddKeys(Handle array, KeyFilter filter); - void PrepareForComparisons(int count); - Handle GetKeys(); + bool AddKey(uint32_t key); + bool AddKey(Object* key, AddKeyConversion convert = DO_NOT_CONVERT); + bool AddKey(Handle key, AddKeyConversion convert = DO_NOT_CONVERT); + void AddKeys(Handle array, + AddKeyConversion convert = DO_NOT_CONVERT); + void AddKeys(Handle array, + AddKeyConversion convert = DO_NOT_CONVERT); + void AddKeysFromProxy(Handle array); + // Jump to the next level, pushing the current |levelLength_| to + // |levelLengths_| and adding a new list to |elements_|. + void NextPrototype(); + // Sort the integer indices in the last list in |elements_| + void SortCurrentElementsList(); + Handle GetKeys(GetKeysConversion convert = KEEP_NUMBERS); - int GetLength() { return length_; } private: - void EnsureCapacity(int capacity); - void Grow(); - Isolate* isolate_; - Handle keys_; - Handle set_; + KeyFilter filter_; + // |elements_| contains the sorted element keys (indices) per level. + List*> elements_; + // |protoLengths_| contains the total number of keys (elements + properties) + // per level. Negative values mark counts for a level with keys from a proxy. + List levelLengths_; + // |properties_| contains the property keys per level in insertion order. + Handle properties_; + // |length_| keeps track of the total number of all element and property keys. int length_; + // |levelLength_| keeps track of the total number of keys + // (elements + properties) in the current level. + int levelLength_; + DISALLOW_COPY_AND_ASSIGN(KeyAccumulator); }; diff --git a/src/runtime/runtime-array.cc b/src/runtime/runtime-array.cc index 349bb62262..0e414384f5 100644 --- a/src/runtime/runtime-array.cc +++ b/src/runtime/runtime-array.cc @@ -206,6 +206,8 @@ RUNTIME_FUNCTION(Runtime_GetArrayKeys) { } KeyAccumulator accumulator(isolate); + // No need to separate protoype levels since we only get numbers/element keys + accumulator.NextPrototype(); for (PrototypeIterator iter(isolate, array, PrototypeIterator::START_AT_RECEIVER); !iter.IsAtEnd(); iter.Advance()) { @@ -217,15 +219,12 @@ RUNTIME_FUNCTION(Runtime_GetArrayKeys) { return *isolate->factory()->NewNumberFromUint(length); } Handle current = PrototypeIterator::GetCurrent(iter); - Handle current_keys = - isolate->factory()->NewFixedArray(current->NumberOfOwnElements(NONE)); - current->GetOwnElementKeys(*current_keys, NONE); - accumulator.AddKeys(current_keys, INCLUDE_SYMBOLS); + JSObject::CollectOwnElementKeys(current, &accumulator, NONE); } // Erase any keys >= length. // TODO(adamk): Remove this step when the contract of %GetArrayKeys // is changed to let this happen on the JS side. - Handle keys = accumulator.GetKeys(); + Handle keys = accumulator.GetKeys(KEEP_NUMBERS); for (int i = 0; i < keys->length(); i++) { if (NumberToUint32(keys->get(i)) >= length) keys->set_undefined(i); } diff --git a/src/runtime/runtime-object.cc b/src/runtime/runtime-object.cc index 53e541a2c8..423d60e5e2 100644 --- a/src/runtime/runtime-object.cc +++ b/src/runtime/runtime-object.cc @@ -981,34 +981,9 @@ RUNTIME_FUNCTION(Runtime_OwnKeys) { Handle contents; ASSIGN_RETURN_FAILURE_ON_EXCEPTION( - isolate, contents, JSReceiver::GetKeys(object, JSReceiver::OWN_ONLY)); - - // Some fast paths through GetKeysInFixedArrayFor reuse a cached - // property array and since the result is mutable we have to create - // a fresh clone on each invocation. - int length = contents->length(); - Handle copy = isolate->factory()->NewFixedArray(length); - int offset = 0; - // Use an outer loop to avoid creating too many handles in the current - // handle scope. - while (offset < length) { - HandleScope scope(isolate); - offset += 100; - int end = Min(offset, length); - for (int i = offset - 100; i < end; i++) { - Object* entry = contents->get(i); - if (entry->IsString()) { - copy->set(i, entry); - } else { - DCHECK(entry->IsNumber()); - Handle entry_handle(entry, isolate); - Handle entry_str = - isolate->factory()->NumberToString(entry_handle); - copy->set(i, *entry_str); - } - } - } - return *isolate->factory()->NewJSArrayWithElements(copy); + isolate, contents, JSReceiver::GetKeys(object, JSReceiver::OWN_ONLY, + SKIP_SYMBOLS, CONVERT_TO_STRING)); + return *isolate->factory()->NewJSArrayWithElements(contents); } diff --git a/test/mjsunit/third_party/object-keys/object-keys.js b/test/mjsunit/third_party/object-keys/object-keys.js index c8003745c4..39336090ea 100644 --- a/test/mjsunit/third_party/object-keys/object-keys.js +++ b/test/mjsunit/third_party/object-keys/object-keys.js @@ -42,6 +42,7 @@ assertEquals(Object.keys({a:null, b:null}), ['a', 'b']); assertEquals(Object.keys({b:null, a:null}), ['b', 'a']); assertEquals(Object.keys([]), []); assertEquals(Object.keys([null]), ['0']); +assertEquals(Object.keys([undefined]), ['0']); assertEquals(Object.keys([null,null]), ['0', '1']); assertEquals(Object.keys([null,null,,,,null]), ['0', '1', '5']); assertEquals(Object.keys({__proto__:{a:null}}), []); @@ -66,3 +67,34 @@ keysBefore[0] = 'x'; var keysAfter = Object.keys(literal); assertEquals(['a', 'b', 'c'], keysAfter); assertEquals(['x', 'b', 'c'], keysBefore); + + +var o = [1, 2, 3]; +assertEquals(['0', '1', '2'], Object.keys(o)); +Object.defineProperty(o, '0', { + enumerable: false, +}); +assertEquals(['1', '2'], Object.keys(o)); + + +(function(){ + assertEquals(['0', '1', '2'], Object.keys(arguments)); + Object.defineProperty(arguments, '0', { + enumerable: false, + }); + assertEquals(['1', '2'], Object.keys(arguments)); +})(0,1,2); + + +(function(a, b){ + assertEquals(['0', '1', '2'], Object.keys(arguments)); + Object.defineProperty(arguments, '0', { + enumerable: false, + }); + assertEquals(['1', '2'], Object.keys(arguments)); +})(0,1,2); + +var b = []; +assertEquals(0, Object.keys(b).length); +b[0] = undefined; +assertEquals(1, Object.keys(b).length);