Commit df8970a8 authored by cbruni's avatar cbruni Committed by Commit bot

[runtime] Fancify KeyAccumulator

Separately collect element keys from property keys to avoid slow
corner-cases. Partly deal with keys generated by Proxies.

BUG=chromium:536790
LOG=N

Review URL: https://codereview.chromium.org/1397063002

Cr-Commit-Position: refs/heads/master@{#31378}
parent 16962756
......@@ -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<JSObject> holder, uint32_t index,
Handle<FixedArrayBase> backing_store) final {
Handle<FixedArrayBase> backing_store,
PropertyAttributes filter) final {
return ElementsAccessorSubclass::HasElementImpl(holder, index,
backing_store);
backing_store, filter);
}
static bool HasElementImpl(Handle<JSObject> holder, uint32_t index,
Handle<FixedArrayBase> backing_store) {
Handle<FixedArrayBase> backing_store,
PropertyAttributes filter) {
return ElementsAccessorSubclass::GetEntryForIndexImpl(
*holder, *backing_store, index) != kMaxUInt32;
*holder, *backing_store, index, filter) != kMaxUInt32;
}
virtual Handle<Object> Get(Handle<FixedArrayBase> 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<JSObject> object,
Handle<FixedArrayBase> 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<JSObject> object,
Handle<FixedArrayBase> 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<JSObject> receiver,
KeyAccumulator* accumulator,
KeyFilter filter) final {
AddKeyConversion convert) final {
Handle<FixedArrayBase> 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<Object> 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<uint32_t>(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<uint32_t>(entry);
}
static PropertyDetails GetDetailsImpl(FixedArrayBase* backing_store,
uint32_t entry) {
return SeededNumberDictionary::cast(backing_store)->DetailsAt(entry);
}
static void CollectElementIndicesImpl(Handle<JSObject> object,
Handle<FixedArrayBase> backing_store,
KeyAccumulator* keys, uint32_t range,
PropertyAttributes filter,
uint32_t offset) {
Handle<SeededNumberDictionary> dictionary =
Handle<SeededNumberDictionary>::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<uint32_t>(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;
}
......
......@@ -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<int>(elements_kind) < kElementsKindCount);
return elements_accessors_[elements_kind];
}
static ElementsAccessor* ForArray(Handle<FixedArrayBase> array);
// Checks the elements of an object for consistency, asserting when a problem
// is found.
virtual void Validate(Handle<JSObject> 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<JSObject> holder, uint32_t index,
Handle<FixedArrayBase> backing_store) = 0;
Handle<FixedArrayBase> backing_store,
PropertyAttributes filter = NONE) = 0;
inline bool HasElement(Handle<JSObject> holder, uint32_t index) {
return HasElement(holder, index, handle(holder->elements()));
inline bool HasElement(Handle<JSObject> 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<JSObject> 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<JSObject> object,
Handle<FixedArrayBase> backing_store,
KeyAccumulator* keys,
uint32_t range = kMaxUInt32,
PropertyAttributes filter = NONE,
uint32_t offset = 0) = 0;
inline void CollectElementIndices(Handle<JSObject> 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<JSObject> receiver,
KeyAccumulator* accumulator,
KeyFilter filter) = 0;
// Returns a shared ElementsAccessor for the specified ElementsKind.
static ElementsAccessor* ForKind(ElementsKind elements_kind) {
DCHECK(static_cast<int>(elements_kind) < kElementsKindCount);
return elements_accessors_[elements_kind];
}
AddKeyConversion convert) = 0;
static ElementsAccessor* ForArray(Handle<FixedArrayBase> array);
virtual void GrowCapacityAndConvert(Handle<JSObject> 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_;
......
......@@ -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> Object::NewStorageFor(Isolate* isolate,
Handle<Object> object,
Representation representation) {
......
This diff is collapsed.
......@@ -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<HeapType> OptimalType(Isolate* isolate, Representation representation);
inline static Handle<Object> 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<FixedArray> GetKeys(
Handle<JSReceiver> 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<JSObject> 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) {}
void AddKey(Handle<Object> key, int check_limit);
void AddKeys(Handle<FixedArray> array, KeyFilter filter);
void AddKeys(Handle<JSObject> array, KeyFilter filter);
void PrepareForComparisons(int count);
Handle<FixedArray> GetKeys();
explicit KeyAccumulator(Isolate* isolate,
KeyFilter filter = KeyFilter::SKIP_SYMBOLS)
: isolate_(isolate), filter_(filter), length_(0), levelLength_(0) {}
~KeyAccumulator();
bool AddKey(uint32_t key);
bool AddKey(Object* key, AddKeyConversion convert = DO_NOT_CONVERT);
bool AddKey(Handle<Object> key, AddKeyConversion convert = DO_NOT_CONVERT);
void AddKeys(Handle<FixedArray> array,
AddKeyConversion convert = DO_NOT_CONVERT);
void AddKeys(Handle<JSObject> array,
AddKeyConversion convert = DO_NOT_CONVERT);
void AddKeysFromProxy(Handle<JSObject> 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<FixedArray> GetKeys(GetKeysConversion convert = KEEP_NUMBERS);
int GetLength() { return length_; }
private:
void EnsureCapacity(int capacity);
void Grow();
Isolate* isolate_;
Handle<FixedArray> keys_;
Handle<OrderedHashSet> set_;
KeyFilter filter_;
// |elements_| contains the sorted element keys (indices) per level.
List<List<uint32_t>*> 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<int> levelLengths_;
// |properties_| contains the property keys per level in insertion order.
Handle<OrderedHashSet> 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);
};
......
......@@ -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<JSObject> current = PrototypeIterator::GetCurrent<JSObject>(iter);
Handle<FixedArray> 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<FixedArray> keys = accumulator.GetKeys();
Handle<FixedArray> keys = accumulator.GetKeys(KEEP_NUMBERS);
for (int i = 0; i < keys->length(); i++) {
if (NumberToUint32(keys->get(i)) >= length) keys->set_undefined(i);
}
......
......@@ -981,34 +981,9 @@ RUNTIME_FUNCTION(Runtime_OwnKeys) {
Handle<FixedArray> 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<FixedArray> 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<Object> entry_handle(entry, isolate);
Handle<Object> 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);
}
......
......@@ -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);
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment