Skip to content

Commit d3324df

Browse files
camillobruniCommit bot
camillobruni
authored and
Commit bot
committed
[keys] Simplify KeyAccumulator
- Use KeyAccumulator::GetKeys directly instead of JSReceiver::GetKeys - Revert KeyAccumulator to single OrderedHashSet implementation. - Convert the OrderedHashSet in-place to a FixedArray - IndexedInterceptor indices are no longer combined and sorted with the object indices BUG= Review-Url: https://codereview.chromium.org/1995263002 Cr-Commit-Position: refs/heads/master@{#36485}
1 parent 6b8d17e commit d3324df

15 files changed

Lines changed: 288 additions & 427 deletions

‎src/api.cc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3872,9 +3872,9 @@ MaybeLocal<Array> v8::Object::GetPropertyNames(Local<Context> context) {
38723872
PREPARE_FOR_EXECUTION(context, Object, GetPropertyNames, Array);
38733873
auto self = Utils::OpenHandle(this);
38743874
i::Handle<i::FixedArray> value;
3875-
has_pending_exception =
3876-
!i::JSReceiver::GetKeys(self, i::INCLUDE_PROTOS, i::ENUMERABLE_STRINGS)
3877-
.ToHandle(&value);
3875+
has_pending_exception = !i::KeyAccumulator::GetKeys(self, i::INCLUDE_PROTOS,
3876+
i::ENUMERABLE_STRINGS)
3877+
.ToHandle(&value);
38783878
RETURN_ON_FAILED_EXECUTION(Array);
38793879
DCHECK(self->map()->EnumLength() == i::kInvalidEnumCacheSentinel ||
38803880
self->map()->EnumLength() == 0 ||
@@ -3905,8 +3905,8 @@ MaybeLocal<Array> v8::Object::GetOwnPropertyNames(Local<Context> context,
39053905
auto self = Utils::OpenHandle(this);
39063906
i::Handle<i::FixedArray> value;
39073907
has_pending_exception =
3908-
!i::JSReceiver::GetKeys(self, i::OWN_ONLY,
3909-
static_cast<i::PropertyFilter>(filter))
3908+
!i::KeyAccumulator::GetKeys(self, i::OWN_ONLY,
3909+
static_cast<i::PropertyFilter>(filter))
39103910
.ToHandle(&value);
39113911
RETURN_ON_FAILED_EXECUTION(Array);
39123912
DCHECK(self->map()->EnumLength() == i::kInvalidEnumCacheSentinel ||

‎src/builtins.cc‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1628,7 +1628,7 @@ BUILTIN(ObjectAssign) {
16281628
Handle<FixedArray> keys;
16291629
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
16301630
isolate, keys,
1631-
JSReceiver::GetKeys(from, OWN_ONLY, ALL_PROPERTIES, KEEP_NUMBERS));
1631+
KeyAccumulator::GetKeys(from, OWN_ONLY, ALL_PROPERTIES, KEEP_NUMBERS));
16321632
// 4c. Repeat for each element nextKey of keys in List order,
16331633
for (int j = 0; j < keys->length(); ++j) {
16341634
Handle<Object> next_key(keys->get(j), isolate);
@@ -1901,7 +1901,7 @@ Object* GetOwnPropertyKeys(Isolate* isolate,
19011901
Handle<FixedArray> keys;
19021902
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
19031903
isolate, keys,
1904-
JSReceiver::GetKeys(receiver, OWN_ONLY, filter, CONVERT_TO_STRING));
1904+
KeyAccumulator::GetKeys(receiver, OWN_ONLY, filter, CONVERT_TO_STRING));
19051905
return *isolate->factory()->NewJSArrayWithElements(keys);
19061906
}
19071907

@@ -1997,8 +1997,8 @@ BUILTIN(ObjectKeys) {
19971997
} else {
19981998
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
19991999
isolate, keys,
2000-
JSReceiver::GetKeys(receiver, OWN_ONLY, ENUMERABLE_STRINGS,
2001-
CONVERT_TO_STRING));
2000+
KeyAccumulator::GetKeys(receiver, OWN_ONLY, ENUMERABLE_STRINGS,
2001+
CONVERT_TO_STRING));
20022002
}
20032003
return *isolate->factory()->NewJSArrayWithElements(keys, FAST_ELEMENTS);
20042004
}
@@ -2040,8 +2040,8 @@ BUILTIN(ObjectGetOwnPropertyDescriptors) {
20402040

20412041
Handle<FixedArray> keys;
20422042
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
2043-
isolate, keys, JSReceiver::GetKeys(receiver, OWN_ONLY, ALL_PROPERTIES,
2044-
CONVERT_TO_STRING));
2043+
isolate, keys, KeyAccumulator::GetKeys(receiver, OWN_ONLY, ALL_PROPERTIES,
2044+
CONVERT_TO_STRING));
20452045

20462046
Handle<JSObject> descriptors =
20472047
isolate->factory()->NewJSObject(isolate->object_function());
@@ -2708,8 +2708,8 @@ BUILTIN(ReflectOwnKeys) {
27082708
Handle<FixedArray> keys;
27092709
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
27102710
isolate, keys,
2711-
JSReceiver::GetKeys(Handle<JSReceiver>::cast(target), OWN_ONLY,
2712-
ALL_PROPERTIES, CONVERT_TO_STRING));
2711+
KeyAccumulator::GetKeys(Handle<JSReceiver>::cast(target), OWN_ONLY,
2712+
ALL_PROPERTIES, CONVERT_TO_STRING));
27132713
return *isolate->factory()->NewJSArrayWithElements(keys);
27142714
}
27152715

‎src/debug/debug-scopes.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -768,7 +768,7 @@ void ScopeIterator::CopyContextExtensionToScopeObject(
768768
if (context->extension_object() == nullptr) return;
769769
Handle<JSObject> extension(context->extension_object());
770770
Handle<FixedArray> keys =
771-
JSReceiver::GetKeys(extension, type, ENUMERABLE_STRINGS)
771+
KeyAccumulator::GetKeys(extension, type, ENUMERABLE_STRINGS)
772772
.ToHandleChecked();
773773

774774
for (int i = 0; i < keys->length(); i++) {

‎src/elements.cc‎

Lines changed: 67 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,26 @@ static void TraceTopFrame(Isolate* isolate) {
445445
JavaScriptFrame::PrintTop(isolate, stdout, false, true);
446446
}
447447

448+
static void SortIndices(
449+
Handle<FixedArray> indices, uint32_t sort_size,
450+
WriteBarrierMode write_barrier_mode = UPDATE_WRITE_BARRIER) {
451+
struct {
452+
bool operator()(Object* a, Object* b) {
453+
if (!a->IsUndefined()) {
454+
if (b->IsUndefined()) return true;
455+
return a->Number() < b->Number();
456+
}
457+
return b->IsUndefined();
458+
}
459+
} cmp;
460+
Object** start =
461+
reinterpret_cast<Object**>(indices->GetFirstElementAddress());
462+
std::sort(start, start + sort_size, cmp);
463+
if (write_barrier_mode != SKIP_WRITE_BARRIER) {
464+
FIXED_ARRAY_ELEMENTS_WRITE_BARRIER(indices->GetIsolate()->heap(), *indices,
465+
0, sort_size);
466+
}
467+
}
448468

449469
// Base class for element handler implementations. Contains the
450470
// the common logic for objects with different ElementsKinds.
@@ -709,8 +729,7 @@ class ElementsAccessorBase : public ElementsAccessor {
709729
JSObject::ValidateElements(array);
710730
}
711731

712-
static uint32_t GetIterationLength(JSObject* receiver,
713-
FixedArrayBase* elements) {
732+
static uint32_t GetMaxIndex(JSObject* receiver, FixedArrayBase* elements) {
714733
if (receiver->IsJSArray()) {
715734
DCHECK(JSArray::cast(receiver)->length()->IsSmi());
716735
return static_cast<uint32_t>(
@@ -719,6 +738,11 @@ class ElementsAccessorBase : public ElementsAccessor {
719738
return Subclass::GetCapacityImpl(receiver, elements);
720739
}
721740

741+
static uint32_t GetMaxNumberOfEntries(JSObject* receiver,
742+
FixedArrayBase* elements) {
743+
return Subclass::GetMaxIndex(receiver, elements);
744+
}
745+
722746
static Handle<FixedArrayBase> ConvertElementsWithCapacity(
723747
Handle<JSObject> object, Handle<FixedArrayBase> old_elements,
724748
ElementsKind from_kind, uint32_t capacity) {
@@ -863,7 +887,6 @@ class ElementsAccessorBase : public ElementsAccessor {
863887
PropertyFilter filter) {
864888
int count = 0;
865889
KeyAccumulator accumulator(isolate, OWN_ONLY, ALL_PROPERTIES);
866-
accumulator.NextPrototype();
867890
Subclass::CollectElementIndicesImpl(
868891
object, handle(object->elements(), isolate), &accumulator);
869892
Handle<FixedArray> keys = accumulator.GetKeys();
@@ -909,11 +932,12 @@ class ElementsAccessorBase : public ElementsAccessor {
909932
KeyAccumulator* keys) {
910933
DCHECK_NE(DICTIONARY_ELEMENTS, kind());
911934
// Non-dictionary elements can't have all-can-read accessors.
912-
uint32_t length = GetIterationLength(*object, *backing_store);
935+
uint32_t length = Subclass::GetMaxIndex(*object, *backing_store);
913936
PropertyFilter filter = keys->filter();
937+
Factory* factory = keys->isolate()->factory();
914938
for (uint32_t i = 0; i < length; i++) {
915939
if (Subclass::HasElementImpl(object, i, backing_store, filter)) {
916-
keys->AddKey(i);
940+
keys->AddKey(factory->NewNumberFromUint(i));
917941
}
918942
}
919943
}
@@ -923,7 +947,7 @@ class ElementsAccessorBase : public ElementsAccessor {
923947
Handle<FixedArrayBase> backing_store, GetKeysConversion convert,
924948
PropertyFilter filter, Handle<FixedArray> list, uint32_t* nof_indices,
925949
uint32_t insertion_index = 0) {
926-
uint32_t length = Subclass::GetIterationLength(*object, *backing_store);
950+
uint32_t length = Subclass::GetMaxIndex(*object, *backing_store);
927951
for (uint32_t i = 0; i < length; i++) {
928952
if (Subclass::HasElementImpl(object, i, backing_store, filter)) {
929953
if (convert == CONVERT_TO_STRING) {
@@ -968,18 +992,7 @@ class ElementsAccessorBase : public ElementsAccessor {
968992

969993
// Sort the indices list if necessary.
970994
if (IsDictionaryElementsKind(kind()) || IsSloppyArgumentsElements(kind())) {
971-
struct {
972-
bool operator()(Object* a, Object* b) {
973-
if (!a->IsUndefined()) {
974-
if (b->IsUndefined()) return true;
975-
return a->Number() < b->Number();
976-
}
977-
return !b->IsUndefined();
978-
}
979-
} cmp;
980-
Object** start =
981-
reinterpret_cast<Object**>(combined_keys->GetFirstElementAddress());
982-
std::sort(start, start + nof_indices, cmp);
995+
SortIndices(combined_keys, nof_indices, SKIP_WRITE_BARRIER);
983996
uint32_t array_length = 0;
984997
// Indices from dictionary elements should only be converted after
985998
// sorting.
@@ -1044,7 +1057,7 @@ class ElementsAccessorBase : public ElementsAccessor {
10441057
? index
10451058
: kMaxUInt32;
10461059
} else {
1047-
uint32_t length = GetIterationLength(holder, backing_store);
1060+
uint32_t length = Subclass::GetMaxIndex(holder, backing_store);
10481061
return index < length ? index : kMaxUInt32;
10491062
}
10501063
}
@@ -1081,17 +1094,15 @@ class DictionaryElementsAccessor
10811094
: ElementsAccessorBase<DictionaryElementsAccessor,
10821095
ElementsKindTraits<DICTIONARY_ELEMENTS> >(name) {}
10831096

1084-
static uint32_t GetIterationLength(JSObject* receiver,
1085-
FixedArrayBase* elements) {
1086-
uint32_t length;
1087-
if (receiver->IsJSArray()) {
1088-
// Special-case GetIterationLength for dictionary elements since the
1089-
// length of the array might be a HeapNumber.
1090-
JSArray::cast(receiver)->length()->ToArrayLength(&length);
1091-
} else {
1092-
length = GetCapacityImpl(receiver, elements);
1093-
}
1094-
return length;
1097+
static uint32_t GetMaxIndex(JSObject* receiver, FixedArrayBase* elements) {
1098+
// We cannot properly estimate this for dictionaries.
1099+
UNREACHABLE();
1100+
}
1101+
1102+
static uint32_t GetMaxNumberOfEntries(JSObject* receiver,
1103+
FixedArrayBase* backing_store) {
1104+
SeededNumberDictionary* dict = SeededNumberDictionary::cast(backing_store);
1105+
return dict->NumberOfElements();
10951106
}
10961107

10971108
static void SetLengthImpl(Isolate* isolate, Handle<JSArray> array,
@@ -1313,21 +1324,27 @@ class DictionaryElementsAccessor
13131324
Handle<FixedArrayBase> backing_store,
13141325
KeyAccumulator* keys) {
13151326
if (keys->filter() & SKIP_STRINGS) return;
1316-
Isolate* isolate = keys->isolate();
1317-
Handle<Object> undefined = isolate->factory()->undefined_value();
1318-
Handle<Object> the_hole = isolate->factory()->the_hole_value();
1327+
Factory* factory = keys->isolate()->factory();
1328+
Handle<Object> undefined = factory->undefined_value();
1329+
Handle<Object> the_hole = factory->the_hole_value();
13191330
Handle<SeededNumberDictionary> dictionary =
13201331
Handle<SeededNumberDictionary>::cast(backing_store);
13211332
int capacity = dictionary->Capacity();
1333+
Handle<FixedArray> elements =
1334+
factory->NewFixedArray(GetMaxNumberOfEntries(*object, *backing_store));
1335+
int insertion_index = 0;
13221336
PropertyFilter filter = keys->filter();
13231337
for (int i = 0; i < capacity; i++) {
13241338
uint32_t key =
13251339
GetKeyForEntryImpl(dictionary, i, filter, *undefined, *the_hole);
13261340
if (key == kMaxUInt32) continue;
1327-
keys->AddKey(key);
1341+
elements->set(insertion_index, *factory->NewNumberFromUint(key));
1342+
insertion_index++;
1343+
}
1344+
SortIndices(elements, insertion_index);
1345+
for (int i = 0; i < insertion_index; i++) {
1346+
keys->AddKey(elements->get(i));
13281347
}
1329-
1330-
keys->SortCurrentElementsList();
13311348
}
13321349

13331350
static Handle<FixedArray> DirectCollectElementIndicesImpl(
@@ -1552,8 +1569,8 @@ class FastElementsAccessor : public ElementsAccessorBase<Subclass, KindTraits> {
15521569
KeyAccumulator* accumulator,
15531570
AddKeyConversion convert) {
15541571
Handle<FixedArrayBase> elements(receiver->elements(),
1555-
receiver->GetIsolate());
1556-
uint32_t length = Subclass::GetIterationLength(*receiver, *elements);
1572+
accumulator->isolate());
1573+
uint32_t length = Subclass::GetMaxNumberOfEntries(*receiver, *elements);
15571574
for (uint32_t i = 0; i < length; i++) {
15581575
if (IsFastPackedElementsKind(KindTraits::Kind) ||
15591576
HasEntryImpl(*elements, i)) {
@@ -2386,18 +2403,16 @@ class SloppyArgumentsElementsAccessor
23862403
static void CollectElementIndicesImpl(Handle<JSObject> object,
23872404
Handle<FixedArrayBase> backing_store,
23882405
KeyAccumulator* keys) {
2389-
FixedArray* parameter_map = FixedArray::cast(*backing_store);
2390-
uint32_t length = parameter_map->length() - 2;
2391-
for (uint32_t i = 0; i < length; ++i) {
2392-
if (!parameter_map->get(i + 2)->IsTheHole()) {
2393-
keys->AddKey(i);
2394-
}
2395-
}
2396-
2397-
Handle<FixedArrayBase> store(FixedArrayBase::cast(parameter_map->get(1)));
2398-
ArgumentsAccessor::CollectElementIndicesImpl(object, store, keys);
2399-
if (Subclass::kind() == FAST_SLOPPY_ARGUMENTS_ELEMENTS) {
2400-
keys->SortCurrentElementsList();
2406+
Isolate* isolate = keys->isolate();
2407+
uint32_t nof_indices = 0;
2408+
Handle<FixedArray> indices = isolate->factory()->NewFixedArray(
2409+
GetCapacityImpl(*object, *backing_store));
2410+
DirectCollectElementIndicesImpl(isolate, object, backing_store,
2411+
KEEP_NUMBERS, ENUMERABLE_STRINGS, indices,
2412+
&nof_indices);
2413+
SortIndices(indices, nof_indices);
2414+
for (uint32_t i = 0; i < nof_indices; i++) {
2415+
keys->AddKey(indices->get(i));
24012416
}
24022417
}
24032418

@@ -2745,8 +2760,9 @@ class StringWrapperElementsAccessor
27452760
Handle<FixedArrayBase> backing_store,
27462761
KeyAccumulator* keys) {
27472762
uint32_t length = GetString(*object)->length();
2763+
Factory* factory = keys->isolate()->factory();
27482764
for (uint32_t i = 0; i < length; i++) {
2749-
keys->AddKey(i);
2765+
keys->AddKey(factory->NewNumberFromUint(i));
27502766
}
27512767
BackingStoreAccessor::CollectElementIndicesImpl(object, backing_store,
27522768
keys);

‎src/json-stringifier.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -544,9 +544,9 @@ BasicJsonStringifier::Result BasicJsonStringifier::SerializeJSReceiverSlow(
544544
if (contents.is_null()) {
545545
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
546546
isolate_, contents,
547-
JSReceiver::GetKeys(object, OWN_ONLY, ENUMERABLE_STRINGS), EXCEPTION);
547+
KeyAccumulator::GetKeys(object, OWN_ONLY, ENUMERABLE_STRINGS),
548+
EXCEPTION);
548549
}
549-
550550
builder_.AppendCharacter('{');
551551
Indent();
552552
bool comma = false;

0 commit comments

Comments
 (0)