Skip to content

Commit 3177928

Browse files
camillobruniCommit bot
camillobruni
authored and
Commit bot
committed
[elements] Enable left-trimming again
Essentially a revert of https://codereview.chromium.org/1346013005 but preserving the refactorings in elements.cc which happened in the mean time. drive-by-fix: pass isolate as argument to more functions in elements.cc. BUG=v8:4606 LOG=y Review URL: https://codereview.chromium.org/1543563002 Cr-Commit-Position: refs/heads/master@{#33023}
1 parent f6d90a6 commit 3177928

2 files changed

Lines changed: 70 additions & 39 deletions

File tree

‎src/elements.cc‎

Lines changed: 69 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -677,11 +677,12 @@ class ElementsAccessorBase : public ElementsAccessor {
677677
}
678678

679679
void SetLength(Handle<JSArray> array, uint32_t length) final {
680-
ElementsAccessorSubclass::SetLengthImpl(array, length,
680+
ElementsAccessorSubclass::SetLengthImpl(array->GetIsolate(), array, length,
681681
handle(array->elements()));
682682
}
683683

684-
static void SetLengthImpl(Handle<JSArray> array, uint32_t length,
684+
static void SetLengthImpl(Isolate* isolate, Handle<JSArray> array,
685+
uint32_t length,
685686
Handle<FixedArrayBase> backing_store) {
686687
DCHECK(!array->SetLengthWouldNormalize(length));
687688
DCHECK(IsFastElementsKind(array->GetElementsKind()));
@@ -698,6 +699,7 @@ class ElementsAccessorBase : public ElementsAccessor {
698699

699700
// Check whether the backing store should be shrunk.
700701
uint32_t capacity = backing_store->length();
702+
old_length = Min(old_length, capacity);
701703
if (length == 0) {
702704
array->initialize_elements();
703705
} else if (length <= capacity) {
@@ -706,7 +708,7 @@ class ElementsAccessorBase : public ElementsAccessor {
706708
}
707709
if (2 * length <= capacity) {
708710
// If more than half the elements won't be used, trim the array.
709-
array->GetHeap()->RightTrimFixedArray<Heap::CONCURRENT_TO_SWEEPER>(
711+
isolate->heap()->RightTrimFixedArray<Heap::CONCURRENT_TO_SWEEPER>(
710712
*backing_store, capacity - length);
711713
} else {
712714
// Otherwise, fill the unused tail with holes.
@@ -972,11 +974,11 @@ class DictionaryElementsAccessor
972974
: ElementsAccessorBase<DictionaryElementsAccessor,
973975
ElementsKindTraits<DICTIONARY_ELEMENTS> >(name) {}
974976

975-
static void SetLengthImpl(Handle<JSArray> array, uint32_t length,
977+
static void SetLengthImpl(Isolate* isolate, Handle<JSArray> array,
978+
uint32_t length,
976979
Handle<FixedArrayBase> backing_store) {
977980
Handle<SeededNumberDictionary> dict =
978981
Handle<SeededNumberDictionary>::cast(backing_store);
979-
Isolate* isolate = array->GetIsolate();
980982
int capacity = dict->Capacity();
981983
uint32_t old_length = 0;
982984
CHECK(array->length()->ToArrayLength(&old_length));
@@ -1352,9 +1354,10 @@ class FastElementsAccessor
13521354
receiver, backing_store, args, unshift_size, AT_START);
13531355
}
13541356

1355-
static void MoveElements(Heap* heap, Handle<FixedArrayBase> backing_store,
1356-
int dst_index, int src_index, int len,
1357-
int hole_start, int hole_end) {
1357+
static void MoveElements(Isolate* isolate, Handle<JSArray> receiver,
1358+
Handle<FixedArrayBase> backing_store, int dst_index,
1359+
int src_index, int len, int hole_start,
1360+
int hole_end) {
13581361
UNREACHABLE();
13591362
}
13601363

@@ -1403,13 +1406,13 @@ class FastElementsAccessor
14031406

14041407
// Delete and move elements to make space for add_count new elements.
14051408
if (add_count < delete_count) {
1406-
FastElementsAccessorSubclass::SpliceShrinkStep(backing_store, heap, start,
1407-
delete_count, add_count,
1408-
length, new_length);
1409+
FastElementsAccessorSubclass::SpliceShrinkStep(
1410+
isolate, receiver, backing_store, start, delete_count, add_count,
1411+
length, new_length);
14091412
} else if (add_count > delete_count) {
14101413
backing_store = FastElementsAccessorSubclass::SpliceGrowStep(
1411-
receiver, backing_store, isolate, heap, start, delete_count,
1412-
add_count, length, new_length);
1414+
isolate, receiver, backing_store, start, delete_count, add_count,
1415+
length, new_length);
14131416
}
14141417

14151418
// Copy over the arguments.
@@ -1423,29 +1426,33 @@ class FastElementsAccessor
14231426
}
14241427

14251428
private:
1426-
static void SpliceShrinkStep(Handle<FixedArrayBase> backing_store, Heap* heap,
1429+
// SpliceShrinkStep might modify the backing_store.
1430+
static void SpliceShrinkStep(Isolate* isolate, Handle<JSArray> receiver,
1431+
Handle<FixedArrayBase> backing_store,
14271432
uint32_t start, uint32_t delete_count,
14281433
uint32_t add_count, uint32_t len,
14291434
uint32_t new_length) {
14301435
const int move_left_count = len - delete_count - start;
14311436
const int move_left_dst_index = start + add_count;
14321437
FastElementsAccessorSubclass::MoveElements(
1433-
heap, backing_store, move_left_dst_index, start + delete_count,
1434-
move_left_count, new_length, len);
1438+
isolate, receiver, backing_store, move_left_dst_index,
1439+
start + delete_count, move_left_count, new_length, len);
14351440
}
14361441

1437-
1442+
// SpliceGrowStep might modify the backing_store.
14381443
static Handle<FixedArrayBase> SpliceGrowStep(
1439-
Handle<JSArray> receiver, Handle<FixedArrayBase> backing_store,
1440-
Isolate* isolate, Heap* heap, uint32_t start, uint32_t delete_count,
1441-
uint32_t add_count, uint32_t length, uint32_t new_length) {
1444+
Isolate* isolate, Handle<JSArray> receiver,
1445+
Handle<FixedArrayBase> backing_store, uint32_t start,
1446+
uint32_t delete_count, uint32_t add_count, uint32_t length,
1447+
uint32_t new_length) {
14421448
// Check we do not overflow the new_length.
14431449
DCHECK((add_count - delete_count) <= (Smi::kMaxValue - length));
14441450
// Check if backing_store is big enough.
14451451
if (new_length <= static_cast<uint32_t>(backing_store->length())) {
14461452
FastElementsAccessorSubclass::MoveElements(
1447-
heap, backing_store, start + add_count, start + delete_count,
1448-
(length - delete_count - start), 0, 0);
1453+
isolate, receiver, backing_store, start + add_count,
1454+
start + delete_count, (length - delete_count - start), 0, 0);
1455+
// MoveElements updates the backing_store in-place.
14491456
return backing_store;
14501457
}
14511458
// New backing storage is needed.
@@ -1466,20 +1473,19 @@ class FastElementsAccessor
14661473
static Handle<Object> RemoveElement(Handle<JSArray> receiver,
14671474
Handle<FixedArrayBase> backing_store,
14681475
Where remove_position) {
1476+
Isolate* isolate = receiver->GetIsolate();
14691477
uint32_t length =
14701478
static_cast<uint32_t>(Smi::cast(receiver->length())->value());
1471-
Isolate* isolate = receiver->GetIsolate();
14721479
DCHECK(length > 0);
14731480
int new_length = length - 1;
14741481
int remove_index = remove_position == AT_START ? 0 : new_length;
14751482
Handle<Object> result =
14761483
FastElementsAccessorSubclass::GetImpl(backing_store, remove_index);
14771484
if (remove_position == AT_START) {
1478-
Heap* heap = isolate->heap();
1479-
FastElementsAccessorSubclass::MoveElements(heap, backing_store, 0, 1,
1480-
new_length, 0, 0);
1485+
FastElementsAccessorSubclass::MoveElements(
1486+
isolate, receiver, backing_store, 0, 1, new_length, 0, 0);
14811487
}
1482-
FastElementsAccessorSubclass::SetLengthImpl(receiver, new_length,
1488+
FastElementsAccessorSubclass::SetLengthImpl(isolate, receiver, new_length,
14831489
backing_store);
14841490

14851491
if (IsHoleyElementsKind(KindTraits::Kind) && result->IsTheHole()) {
@@ -1513,8 +1519,8 @@ class FastElementsAccessor
15131519
// If the backing store has enough capacity and we add elements to the
15141520
// start we have to shift the existing objects.
15151521
Isolate* isolate = receiver->GetIsolate();
1516-
FastElementsAccessorSubclass::MoveElements(isolate->heap(), backing_store,
1517-
add_size, 0, length, 0, 0);
1522+
FastElementsAccessorSubclass::MoveElements(
1523+
isolate, receiver, backing_store, add_size, 0, length, 0, 0);
15181524
}
15191525

15201526
int insertion_index = remove_position == AT_START ? 0 : length;
@@ -1567,11 +1573,22 @@ class FastSmiOrObjectElementsAccessor
15671573
return backing_store->get(index);
15681574
}
15691575

1570-
static void MoveElements(Heap* heap, Handle<FixedArrayBase> backing_store,
1571-
int dst_index, int src_index, int len,
1572-
int hole_start, int hole_end) {
1576+
static void MoveElements(Isolate* isolate, Handle<JSArray> receiver,
1577+
Handle<FixedArrayBase> backing_store, int dst_index,
1578+
int src_index, int len, int hole_start,
1579+
int hole_end) {
1580+
Heap* heap = isolate->heap();
15731581
Handle<FixedArray> dst_elms = Handle<FixedArray>::cast(backing_store);
1574-
if (len != 0) {
1582+
if (heap->CanMoveObjectStart(*dst_elms) && dst_index == 0) {
1583+
// Update all the copies of this backing_store handle.
1584+
*dst_elms.location() =
1585+
FixedArray::cast(heap->LeftTrimFixedArray(*dst_elms, src_index));
1586+
receiver->set_elements(*dst_elms);
1587+
// Adjust the hole offset as the array has been shrunk.
1588+
hole_end -= src_index;
1589+
DCHECK_LE(hole_start, backing_store->length());
1590+
DCHECK_LE(hole_end, backing_store->length());
1591+
} else if (len != 0) {
15751592
DisallowHeapAllocation no_gc;
15761593
heap->MoveElements(*dst_elms, dst_index, src_index, len);
15771594
}
@@ -1690,12 +1707,23 @@ class FastDoubleElementsAccessor
16901707
FixedDoubleArray::cast(backing_store)->set(entry, value->Number());
16911708
}
16921709

1693-
static void MoveElements(Heap* heap, Handle<FixedArrayBase> backing_store,
1694-
int dst_index, int src_index, int len,
1695-
int hole_start, int hole_end) {
1710+
static void MoveElements(Isolate* isolate, Handle<JSArray> receiver,
1711+
Handle<FixedArrayBase> backing_store, int dst_index,
1712+
int src_index, int len, int hole_start,
1713+
int hole_end) {
1714+
Heap* heap = isolate->heap();
16961715
Handle<FixedDoubleArray> dst_elms =
16971716
Handle<FixedDoubleArray>::cast(backing_store);
1698-
if (len != 0) {
1717+
if (heap->CanMoveObjectStart(*dst_elms) && dst_index == 0) {
1718+
// Update all the copies of this backing_store handle.
1719+
*dst_elms.location() = FixedDoubleArray::cast(
1720+
heap->LeftTrimFixedArray(*dst_elms, src_index));
1721+
receiver->set_elements(*dst_elms);
1722+
// Adjust the hole offset as the array has been shrunk.
1723+
hole_end -= src_index;
1724+
DCHECK_LE(hole_start, backing_store->length());
1725+
DCHECK_LE(hole_end, backing_store->length());
1726+
} else if (len != 0) {
16991727
MemMove(dst_elms->data_start() + dst_index,
17001728
dst_elms->data_start() + src_index, len * kDoubleSize);
17011729
}
@@ -1801,7 +1829,8 @@ class TypedElementsAccessor
18011829
return PropertyDetails(DONT_DELETE, DATA, 0, PropertyCellType::kNoCell);
18021830
}
18031831

1804-
static void SetLengthImpl(Handle<JSArray> array, uint32_t length,
1832+
static void SetLengthImpl(Isolate* isolate, Handle<JSArray> array,
1833+
uint32_t length,
18051834
Handle<FixedArrayBase> backing_store) {
18061835
// External arrays do not support changing their length.
18071836
UNREACHABLE();
@@ -1915,7 +1944,8 @@ class SloppyArgumentsElementsAccessor
19151944
}
19161945
}
19171946

1918-
static void SetLengthImpl(Handle<JSArray> array, uint32_t length,
1947+
static void SetLengthImpl(Isolate* isolate, Handle<JSArray> array,
1948+
uint32_t length,
19191949
Handle<FixedArrayBase> parameter_map) {
19201950
// Sloppy arguments objects are not arrays.
19211951
UNREACHABLE();

‎src/flag-definitions.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ DEFINE_NEG_IMPLICATION(harmony, promise_extra)
195195

196196
// Activate on ClusterFuzz.
197197
DEFINE_IMPLICATION(es_staging, harmony_regexp_lookbehind)
198+
DEFINE_IMPLICATION(es_staging, move_object_start)
198199

199200
// Features that are still work in progress (behind individual flags).
200201
#define HARMONY_INPROGRESS(V) \

0 commit comments

Comments
 (0)