Skip to content

Commit 8076d6e

Browse files
verwaestCommit bot
verwaest
authored and
Commit bot
committed
More cleanup related to setting array.length
BUG= Review URL: https://codereview.chromium.org/1191313003 Cr-Commit-Position: refs/heads/master@{#29152}
1 parent c166945 commit 8076d6e

10 files changed

Lines changed: 107 additions & 147 deletions

File tree

‎src/accessors.cc‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ void Accessors::ArrayLengthSetter(
204204
v8::Local<v8::Name> name,
205205
v8::Local<v8::Value> val,
206206
const v8::PropertyCallbackInfo<void>& info) {
207+
// TODO(verwaest): Speed up.
207208
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
208209
HandleScope scope(isolate);
209210
Handle<JSObject> object = Utils::OpenHandle(*info.This());
@@ -227,7 +228,9 @@ void Accessors::ArrayLengthSetter(
227228
}
228229

229230
if (uint32_v->Number() == number_v->Number()) {
230-
maybe = JSArray::SetElementsLength(array_handle, uint32_v);
231+
uint32_t new_length = 0;
232+
CHECK(uint32_v->ToArrayLength(&new_length));
233+
maybe = JSArray::ObservableSetLength(array_handle, new_length);
231234
if (maybe.is_null()) isolate->OptionalRescheduleException(false);
232235
return;
233236
}

‎src/ast.cc‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -538,8 +538,7 @@ void ArrayLiteral::BuildConstantElements(Isolate* isolate) {
538538
}
539539

540540
if (array_index != values()->length()) {
541-
JSArray::SetElementsLength(
542-
array, handle(Smi::FromInt(array_index), isolate)).Assert();
541+
JSArray::SetLength(array, array_index);
543542
}
544543
Handle<FixedArrayBase> element_values(array->elements());
545544

‎src/builtins.cc‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -445,15 +445,12 @@ BUILTIN(ArrayPop) {
445445
return CallJsBuiltin(isolate, "$arrayPop", args);
446446
}
447447

448-
ElementsAccessor* accessor = array->GetElementsAccessor();
449448
uint32_t new_length = len - 1;
450449
Handle<Object> element;
451450
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
452451
isolate, element, Object::GetElement(isolate, array, new_length));
453452

454-
RETURN_FAILURE_ON_EXCEPTION(
455-
isolate,
456-
accessor->SetLength(array, handle(Smi::FromInt(new_length), isolate)));
453+
JSArray::SetLength(array, new_length);
457454
return *element;
458455
}
459456

‎src/elements.cc‎

Lines changed: 67 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -636,16 +636,13 @@ class ElementsAccessorBase : public ElementsAccessor {
636636
return MaybeHandle<AccessorPair>();
637637
}
638638

639-
MUST_USE_RESULT virtual MaybeHandle<Object> SetLength(
640-
Handle<JSArray> array, Handle<Object> length) final {
641-
return ElementsAccessorSubclass::SetLengthImpl(
642-
array, length, handle(array->elements()));
639+
virtual void SetLength(Handle<JSArray> array, uint32_t length) final {
640+
ElementsAccessorSubclass::SetLengthImpl(array, length,
641+
handle(array->elements()));
643642
}
644643

645-
MUST_USE_RESULT static MaybeHandle<Object> SetLengthImpl(
646-
Handle<JSObject> obj,
647-
Handle<Object> length,
648-
Handle<FixedArrayBase> backing_store);
644+
static void SetLengthImpl(Handle<JSArray> array, uint32_t length,
645+
Handle<FixedArrayBase> backing_store);
649646

650647
virtual void SetCapacityAndLength(Handle<JSArray> array, int capacity,
651648
int length) final {
@@ -866,10 +863,8 @@ class FastElementsAccessor
866863
typedef typename KindTraits::BackingStore BackingStore;
867864

868865
// Adjusts the length of the fast backing store.
869-
static Handle<Object> SetLengthWithoutNormalize(
870-
Handle<FixedArrayBase> backing_store,
871-
Handle<JSArray> array,
872-
Handle<Object> length_object,
866+
static uint32_t SetLengthWithoutNormalize(
867+
Handle<FixedArrayBase> backing_store, Handle<JSArray> array,
873868
uint32_t length) {
874869
Isolate* isolate = array->GetIsolate();
875870
uint32_t old_capacity = backing_store->length();
@@ -904,7 +899,7 @@ class FastElementsAccessor
904899
Handle<BackingStore>::cast(backing_store)->set_the_hole(i);
905900
}
906901
}
907-
return length_object;
902+
return length;
908903
}
909904

910905
// Check whether the backing store should be expanded.
@@ -913,7 +908,7 @@ class FastElementsAccessor
913908
FastElementsAccessorSubclass::SetFastElementsCapacityAndLength(
914909
array, new_capacity, length);
915910
JSObject::ValidateElements(array);
916-
return length_object;
911+
return length;
917912
}
918913

919914
static void DeleteCommon(Handle<JSObject> obj, uint32_t key,
@@ -1271,13 +1266,10 @@ class TypedElementsAccessor
12711266
return PropertyDetails(DONT_DELETE, DATA, 0, PropertyCellType::kNoCell);
12721267
}
12731268

1274-
MUST_USE_RESULT static MaybeHandle<Object> SetLengthImpl(
1275-
Handle<JSObject> obj,
1276-
Handle<Object> length,
1277-
Handle<FixedArrayBase> backing_store) {
1269+
static void SetLengthImpl(Handle<JSArray> array, uint32_t length,
1270+
Handle<FixedArrayBase> backing_store) {
12781271
// External arrays do not support changing their length.
12791272
UNREACHABLE();
1280-
return obj;
12811273
}
12821274

12831275
virtual void Delete(Handle<JSObject> obj, uint32_t key,
@@ -1327,39 +1319,51 @@ class DictionaryElementsAccessor
13271319
: ElementsAccessorBase<DictionaryElementsAccessor,
13281320
ElementsKindTraits<DICTIONARY_ELEMENTS> >(name) {}
13291321

1322+
static void SetLengthImpl(Handle<JSArray> array, uint32_t length,
1323+
Handle<FixedArrayBase> backing_store) {
1324+
uint32_t new_length =
1325+
SetLengthWithoutNormalize(backing_store, array, length);
1326+
// SetLengthWithoutNormalize does not allow length to drop below the last
1327+
// non-deletable element.
1328+
DCHECK_GE(new_length, length);
1329+
if (new_length <= Smi::kMaxValue) {
1330+
array->set_length(Smi::FromInt(new_length));
1331+
} else {
1332+
Isolate* isolate = array->GetIsolate();
1333+
Handle<Object> length_obj =
1334+
isolate->factory()->NewNumberFromUint(new_length);
1335+
array->set_length(*length_obj);
1336+
}
1337+
}
1338+
13301339
// Adjusts the length of the dictionary backing store and returns the new
13311340
// length according to ES5 section 15.4.5.2 behavior.
1332-
static Handle<Object> SetLengthWithoutNormalize(
1333-
Handle<FixedArrayBase> store,
1334-
Handle<JSArray> array,
1335-
Handle<Object> length_object,
1336-
uint32_t length) {
1341+
static uint32_t SetLengthWithoutNormalize(Handle<FixedArrayBase> store,
1342+
Handle<JSArray> array,
1343+
uint32_t length) {
13371344
Handle<SeededNumberDictionary> dict =
13381345
Handle<SeededNumberDictionary>::cast(store);
13391346
Isolate* isolate = array->GetIsolate();
13401347
int capacity = dict->Capacity();
1341-
uint32_t new_length = length;
1342-
uint32_t old_length = static_cast<uint32_t>(array->length()->Number());
1343-
if (new_length < old_length) {
1348+
uint32_t old_length = 0;
1349+
CHECK(array->length()->ToArrayLength(&old_length));
1350+
if (length < old_length) {
13441351
// Find last non-deletable element in range of elements to be
13451352
// deleted and adjust range accordingly.
13461353
for (int i = 0; i < capacity; i++) {
13471354
DisallowHeapAllocation no_gc;
13481355
Object* key = dict->KeyAt(i);
13491356
if (key->IsNumber()) {
13501357
uint32_t number = static_cast<uint32_t>(key->Number());
1351-
if (new_length <= number && number < old_length) {
1358+
if (length <= number && number < old_length) {
13521359
PropertyDetails details = dict->DetailsAt(i);
1353-
if (!details.IsConfigurable()) new_length = number + 1;
1360+
if (!details.IsConfigurable()) length = number + 1;
13541361
}
13551362
}
13561363
}
1357-
if (new_length != length) {
1358-
length_object = isolate->factory()->NewNumberFromUint(new_length);
1359-
}
13601364
}
13611365

1362-
if (new_length == 0) {
1366+
if (length == 0) {
13631367
// Flush the backing store.
13641368
JSObject::ResetElements(array);
13651369
} else {
@@ -1371,7 +1375,7 @@ class DictionaryElementsAccessor
13711375
Object* key = dict->KeyAt(i);
13721376
if (key->IsNumber()) {
13731377
uint32_t number = static_cast<uint32_t>(key->Number());
1374-
if (new_length <= number && number < old_length) {
1378+
if (length <= number && number < old_length) {
13751379
dict->SetEntry(i, the_hole_value, the_hole_value);
13761380
removed_entries++;
13771381
}
@@ -1381,7 +1385,7 @@ class DictionaryElementsAccessor
13811385
// Update the number of elements.
13821386
dict->ElementsRemoved(removed_entries);
13831387
}
1384-
return length_object;
1388+
return length;
13851389
}
13861390

13871391
static void DeleteCommon(Handle<JSObject> obj, uint32_t key,
@@ -1566,14 +1570,10 @@ class SloppyArgumentsElementsAccessor : public ElementsAccessorBase<
15661570
}
15671571
}
15681572

1569-
MUST_USE_RESULT static MaybeHandle<Object> SetLengthImpl(
1570-
Handle<JSObject> obj,
1571-
Handle<Object> length,
1572-
Handle<FixedArrayBase> parameter_map) {
1573-
// TODO(mstarzinger): This was never implemented but will be used once we
1574-
// correctly implement [[DefineOwnProperty]] on arrays.
1575-
UNIMPLEMENTED();
1576-
return obj;
1573+
static void SetLengthImpl(Handle<JSArray> array, uint32_t length,
1574+
Handle<FixedArrayBase> parameter_map) {
1575+
// Sloppy arguments objects are not arrays.
1576+
UNREACHABLE();
15771577
}
15781578

15791579
virtual void Delete(Handle<JSObject> obj, uint32_t key,
@@ -1704,52 +1704,26 @@ void ElementsAccessor::TearDown() {
17041704

17051705

17061706
template <typename ElementsAccessorSubclass, typename ElementsKindTraits>
1707-
MUST_USE_RESULT MaybeHandle<Object> ElementsAccessorBase<
1708-
ElementsAccessorSubclass,
1709-
ElementsKindTraits>::SetLengthImpl(Handle<JSObject> obj,
1710-
Handle<Object> length_obj,
1711-
Handle<FixedArrayBase> backing_store) {
1712-
Handle<JSArray> array = Handle<JSArray>::cast(obj);
1713-
1714-
uint32_t length = 0;
1715-
CHECK(length_obj->ToArrayLength(&length));
1716-
// Fast case: length fits in a smi.
1717-
if (length <= Smi::kMaxValue) {
1718-
Handle<Smi> smi(Smi::FromInt(length), obj->GetIsolate());
1719-
Handle<Object> new_length =
1720-
ElementsAccessorSubclass::SetLengthWithoutNormalize(backing_store,
1721-
array, smi, length);
1722-
DCHECK(!new_length.is_null());
1723-
1724-
// Even though the proposed length was a smi, new_length could
1725-
// still be a heap number because SetLengthWithoutNormalize doesn't
1726-
// allow the array length property to drop below the index of
1727-
// non-deletable elements.
1728-
DCHECK(new_length->IsSmi() || new_length->IsHeapNumber() ||
1729-
new_length->IsUndefined());
1730-
if (new_length->IsSmi()) {
1731-
array->set_length(*Handle<Smi>::cast(new_length));
1732-
return array;
1733-
} else if (new_length->IsHeapNumber()) {
1734-
array->set_length(*new_length);
1735-
return array;
1736-
}
1707+
void ElementsAccessorBase<ElementsAccessorSubclass, ElementsKindTraits>::
1708+
SetLengthImpl(Handle<JSArray> array, uint32_t length,
1709+
Handle<FixedArrayBase> backing_store) {
1710+
// Normalize if the length does not fit in a smi. Fast mode arrays only
1711+
// support smi length.
1712+
if (JSArray::SetLengthWouldNormalize(array->GetHeap(), length)) {
1713+
Handle<SeededNumberDictionary> dictionary =
1714+
JSObject::NormalizeElements(array);
1715+
DCHECK(!dictionary.is_null());
1716+
DictionaryElementsAccessor::SetLengthImpl(array, length, dictionary);
1717+
} else {
1718+
#ifdef DEBUG
1719+
uint32_t max = Smi::kMaxValue;
1720+
DCHECK_LE(length, max);
1721+
#endif
1722+
uint32_t new_length = ElementsAccessorSubclass::SetLengthWithoutNormalize(
1723+
backing_store, array, length);
1724+
DCHECK_EQ(length, new_length);
1725+
array->set_length(Smi::FromInt(new_length));
17371726
}
1738-
1739-
// Slow case: The new length does not fit into a Smi or conversion
1740-
// to slow elements is needed for other reasons.
1741-
Handle<SeededNumberDictionary> dictionary =
1742-
JSObject::NormalizeElements(array);
1743-
DCHECK(!dictionary.is_null());
1744-
1745-
Handle<Object> new_length =
1746-
DictionaryElementsAccessor::SetLengthWithoutNormalize(dictionary, array,
1747-
length_obj, length);
1748-
DCHECK(!new_length.is_null());
1749-
1750-
DCHECK(new_length->IsNumber());
1751-
array->set_length(*new_length);
1752-
return array;
17531727
}
17541728

17551729

@@ -1776,15 +1750,14 @@ MaybeHandle<Object> ArrayConstructInitializeElements(Handle<JSArray> array,
17761750
elements_kind = GetHoleyElementsKind(elements_kind);
17771751
JSObject::TransitionElementsKind(array, elements_kind);
17781752
}
1779-
return array;
17801753
} else if (length == 0) {
17811754
JSArray::Initialize(array, JSArray::kPreallocatedArrayElements);
1782-
return array;
1755+
} else {
1756+
// Take the argument as the length.
1757+
JSArray::Initialize(array, 0);
1758+
JSArray::SetLength(array, length);
17831759
}
1784-
1785-
// Take the argument as the length.
1786-
JSArray::Initialize(array, 0);
1787-
return JSArray::SetElementsLength(array, args->at<Object>(0));
1760+
return array;
17881761
}
17891762

17901763
Factory* factory = array->GetIsolate()->factory();

‎src/elements.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,7 @@ class ElementsAccessor {
7474
// changing array sizes as defined in EcmaScript 5.1 15.4.5.2, i.e. array that
7575
// have non-deletable elements can only be shrunk to the size of highest
7676
// element that is non-deletable.
77-
MUST_USE_RESULT virtual MaybeHandle<Object> SetLength(
78-
Handle<JSArray> holder,
79-
Handle<Object> new_length) = 0;
77+
virtual void SetLength(Handle<JSArray> holder, uint32_t new_length) = 0;
8078

8179
// Modifies both the length and capacity of a JSArray, resizing the underlying
8280
// backing store as necessary. This method does NOT honor the semantics of

‎src/objects-inl.h‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6970,18 +6970,16 @@ void JSArray::set_length(Smi* length) {
69706970
}
69716971

69726972

6973-
bool JSArray::SetElementsLengthWouldNormalize(
6974-
Heap* heap, Handle<Object> new_length_handle) {
6973+
bool JSArray::SetLengthWouldNormalize(Heap* heap, uint32_t new_length) {
69756974
// If the new array won't fit in a some non-trivial fraction of the max old
69766975
// space size, then force it to go dictionary mode.
6977-
int max_fast_array_size =
6978-
static_cast<int>((heap->MaxOldGenerationSize() / kDoubleSize) / 4);
6979-
return new_length_handle->IsNumber() &&
6980-
NumberToInt32(*new_length_handle) >= max_fast_array_size;
6976+
uint32_t max_fast_array_size =
6977+
static_cast<uint32_t>((heap->MaxOldGenerationSize() / kDoubleSize) / 4);
6978+
return new_length >= max_fast_array_size;
69816979
}
69826980

69836981

6984-
bool JSArray::AllowsSetElementsLength() {
6982+
bool JSArray::AllowsSetLength() {
69856983
bool result = elements()->IsFixedArray() || elements()->IsFixedDoubleArray();
69866984
DCHECK(result == !HasExternalArrayElements());
69876985
return result;

0 commit comments

Comments
 (0)