Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(554)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: src/elements.cc

Issue 1346013005: elements.cc cleanup (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: addressing nits Created 5 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/elements.cc
diff --git a/src/elements.cc b/src/elements.cc
index 5cf248c95245ee6f09188725f372ceacbe81813c..901844bc1839d742f46376e5d0e1a13fad2bae88 100644
--- a/src/elements.cc
+++ b/src/elements.cc
@@ -51,6 +51,8 @@ namespace {
static const int kPackedSizeNotKnown = -1;
+enum Where { AT_START, AT_END };
+
// First argument in list is the accessor class, the second argument is the
// accessor ElementsKind, and the third is the backing store class. Use the
@@ -1338,119 +1340,28 @@ class FastElementsAccessor
static Handle<Object> PopImpl(Handle<JSArray> receiver,
Handle<FixedArrayBase> backing_store) {
- uint32_t len =
- static_cast<uint32_t>(Smi::cast(receiver->length())->value());
- DCHECK(len > 0);
- uint32_t new_length = len - 1;
- Handle<Object> result =
- FastElementsAccessorSubclass::GetImpl(backing_store, new_length);
- FastElementsAccessorSubclass::SetLengthImpl(receiver, new_length,
- backing_store);
-
- if (IsHoleyElementsKind(KindTraits::Kind) && result->IsTheHole()) {
- return receiver->GetIsolate()->factory()->undefined_value();
- }
- return result;
+ return FastElementsAccessorSubclass::RemoveElement(receiver, backing_store,
+ AT_END);
}
static Handle<Object> ShiftImpl(Handle<JSArray> receiver,
Handle<FixedArrayBase> backing_store) {
- uint32_t len =
- static_cast<uint32_t>(Smi::cast(receiver->length())->value());
- Isolate* isolate = receiver->GetIsolate();
- DCHECK(len > 0);
- int new_length = len - 1;
- Handle<Object> result =
- FastElementsAccessorSubclass::GetImpl(backing_store, 0);
- Heap* heap = isolate->heap();
- if (heap->CanMoveObjectStart(*backing_store)) {
- receiver->set_elements(heap->LeftTrimFixedArray(*backing_store, 1));
- } else {
- FastElementsAccessorSubclass::MoveElements(heap, backing_store, 0, 1,
- new_length, 0, 0);
- }
- FastElementsAccessorSubclass::SetLengthImpl(receiver, new_length,
- backing_store);
-
- if (IsHoleyElementsKind(KindTraits::Kind) && result->IsTheHole()) {
- result = receiver->GetIsolate()->factory()->undefined_value();
- }
- return result;
+ return FastElementsAccessorSubclass::RemoveElement(receiver, backing_store,
+ AT_START);
}
static uint32_t PushImpl(Handle<JSArray> receiver,
Handle<FixedArrayBase> backing_store,
Arguments* args, uint32_t push_size) {
- uint32_t len = Smi::cast(receiver->length())->value();
- DCHECK(push_size > 0);
- uint32_t elms_len = backing_store->length();
- // Currently fixed arrays cannot grow too big, so
- // we should never hit this case.
- DCHECK(push_size <= static_cast<uint32_t>(Smi::kMaxValue - len));
- uint32_t new_length = len + push_size;
-
- if (new_length > elms_len) {
- // New backing storage is needed.
- uint32_t capacity = new_length + (new_length >> 1) + 16;
- backing_store = FastElementsAccessorSubclass::ConvertElementsWithCapacity(
- receiver, backing_store, KindTraits::Kind, capacity);
- receiver->set_elements(*backing_store);
- }
-
- // Add the provided values.
- DisallowHeapAllocation no_gc;
- FixedArrayBase* raw_backing_store = *backing_store;
- WriteBarrierMode mode = raw_backing_store->GetWriteBarrierMode(no_gc);
- for (uint32_t index = 0; index < push_size; index++) {
- Object* object = (*args)[index + 1];
- FastElementsAccessorSubclass::SetImpl(raw_backing_store, index + len,
- object, mode);
- }
- DCHECK(*backing_store == receiver->elements());
- // Set the length.
- receiver->set_length(Smi::FromInt(new_length));
- return new_length;
+ return FastElementsAccessorSubclass::AddArguments(receiver, backing_store,
+ args, push_size, AT_END);
}
static uint32_t UnshiftImpl(Handle<JSArray> receiver,
Handle<FixedArrayBase> backing_store,
Arguments* args, uint32_t unshift_size) {
- uint32_t len = Smi::cast(receiver->length())->value();
- DCHECK(unshift_size > 0);
- uint32_t elms_len = backing_store->length();
- // Currently fixed arrays cannot grow too big, so
- // we should never hit this case.
- DCHECK(unshift_size <= static_cast<uint32_t>(Smi::kMaxValue - len));
- uint32_t new_length = len + unshift_size;
-
- if (new_length > elms_len) {
- // New backing storage is needed.
- uint32_t capacity = new_length + (new_length >> 1) + 16;
- backing_store = FastElementsAccessorSubclass::ConvertElementsWithCapacity(
- receiver, backing_store, KindTraits::Kind, capacity, 0, unshift_size,
- ElementsAccessor::kCopyToEndAndInitializeToHole);
- DisallowHeapAllocation no_gc;
- receiver->set_elements(*backing_store);
- } else {
- // unshift_size is > 0 and new_length <= elms_len, so backing_store cannot
- // be the empty_fixed_array.
- DisallowHeapAllocation no_gc;
- Isolate* isolate = receiver->GetIsolate();
- FastElementsAccessorSubclass::MoveElements(isolate->heap(), backing_store,
- unshift_size, 0, len, 0, 0);
- }
-
- // Add the provided values.
- DisallowHeapAllocation no_gc;
- FixedArrayBase* raw_backing_store = *backing_store;
- WriteBarrierMode mode = raw_backing_store->GetWriteBarrierMode(no_gc);
- for (uint32_t index = 0; index < unshift_size; index++) {
- FastElementsAccessorSubclass::SetImpl(raw_backing_store, index,
- (*args)[index + 1], mode);
- }
- // Set the length.
- receiver->set_length(Smi::FromInt(new_length));
- return new_length;
+ return FastElementsAccessorSubclass::AddArguments(
+ receiver, backing_store, args, unshift_size, AT_START);
}
static void MoveElements(Heap* heap, Handle<FixedArrayBase> backing_store,
@@ -1482,8 +1393,8 @@ class FastElementsAccessor
Arguments* args, uint32_t add_count) {
Isolate* isolate = receiver->GetIsolate();
Heap* heap = isolate->heap();
- uint32_t len = Smi::cast(receiver->length())->value();
- uint32_t new_length = len - delete_count + add_count;
+ uint32_t length = Smi::cast(receiver->length())->value();
+ uint32_t new_length = length - delete_count + add_count;
if (new_length == 0) {
receiver->set_elements(heap->empty_fixed_array());
@@ -1492,7 +1403,7 @@ class FastElementsAccessor
backing_store, KindTraits::Kind, delete_count);
}
- // construct the result array which holds the deleted elements
+ // Construct the result array which holds the deleted elements.
Handle<JSArray> deleted_elements = isolate->factory()->NewJSArray(
KindTraits::Kind, delete_count, delete_count);
if (delete_count > 0) {
@@ -1502,30 +1413,21 @@ class FastElementsAccessor
0, kPackedSizeNotKnown, delete_count);
}
- // delete and move elements to make space for add_count new elements
- bool elms_changed = false;
+ // Delete and move elements to make space for add_count new elements.
if (add_count < delete_count) {
- elms_changed = SpliceShrinkStep(backing_store, heap, start, delete_count,
- add_count, len, new_length);
+ FastElementsAccessorSubclass::SpliceShrinkStep(backing_store, heap, start,
+ delete_count, add_count,
+ length, new_length);
} else if (add_count > delete_count) {
- elms_changed =
- SpliceGrowStep(receiver, backing_store, isolate, heap, start,
- delete_count, add_count, len, new_length);
+ backing_store = FastElementsAccessorSubclass::SpliceGrowStep(
+ receiver, backing_store, isolate, heap, start, delete_count,
+ add_count, length, new_length);
}
- // Copy new Elements from args
- DisallowHeapAllocation no_gc;
- FixedArrayBase* raw_backing_store = *backing_store;
- WriteBarrierMode mode = raw_backing_store->GetWriteBarrierMode(no_gc);
- for (uint32_t index = 0; index < add_count; index++) {
- Object* object = (*args)[3 + index];
- FastElementsAccessorSubclass::SetImpl(raw_backing_store, index + start,
- object, mode);
- }
+ // Copy over the arguments.
+ FastElementsAccessorSubclass::CopyArguments(args, backing_store, add_count,
+ 3, start);
- if (elms_changed) {
- receiver->set_elements(*backing_store);
- }
receiver->set_length(Smi::FromInt(new_length));
FastElementsAccessorSubclass::TryTransitionResultArrayToPacked(
deleted_elements);
@@ -1533,63 +1435,121 @@ class FastElementsAccessor
}
private:
- static bool SpliceShrinkStep(Handle<FixedArrayBase>& backing_store,
- Heap* heap, uint32_t start,
- uint32_t delete_count, uint32_t add_count,
- uint32_t len, uint32_t new_length) {
+ static void SpliceShrinkStep(Handle<FixedArrayBase> backing_store, Heap* heap,
+ uint32_t start, uint32_t delete_count,
+ uint32_t add_count, uint32_t len,
+ uint32_t new_length) {
const int move_left_count = len - delete_count - start;
const int move_left_dst_index = start + add_count;
- const bool left_trim_array = heap->CanMoveObjectStart(*backing_store) &&
- (move_left_dst_index < move_left_count);
- if (left_trim_array) {
- const int delta = delete_count - add_count;
- // shift from before the insertion point to the right
- FastElementsAccessorSubclass::MoveElements(heap, backing_store, delta, 0,
- start, 0, 0);
- backing_store = handle(heap->LeftTrimFixedArray(*backing_store, delta));
- return true;
- } else {
- // No left-trim needed or possible (in this case we left-move and store
- // the hole)
+ FastElementsAccessorSubclass::MoveElements(
+ heap, backing_store, move_left_dst_index, start + delete_count,
+ move_left_count, new_length, len);
+ }
+
+
+ static Handle<FixedArrayBase> SpliceGrowStep(
+ Handle<JSArray> receiver, Handle<FixedArrayBase> backing_store,
+ Isolate* isolate, Heap* heap, uint32_t start, uint32_t delete_count,
+ uint32_t add_count, uint32_t length, uint32_t new_length) {
+ // Check we do not overflow the new_length.
+ DCHECK((add_count - delete_count) <= (Smi::kMaxValue - length));
+ // Check if backing_store is big enough.
+ if (new_length <= static_cast<uint32_t>(backing_store->length())) {
FastElementsAccessorSubclass::MoveElements(
- heap, backing_store, move_left_dst_index, start + delete_count,
- move_left_count, new_length, len);
+ heap, backing_store, start + add_count, start + delete_count,
+ (length - delete_count - start), 0, 0);
+ return backing_store;
+ }
+ // New backing storage is needed.
+ int capacity = JSObject::NewElementsCapacity(new_length);
+ // Partially copy all elements up to start.
+ Handle<FixedArrayBase> new_elms =
+ FastElementsAccessorSubclass::ConvertElementsWithCapacity(
+ receiver, backing_store, KindTraits::Kind, capacity, start);
+ // Copy the trailing elements after start + delete_count
+ FastElementsAccessorSubclass::CopyElementsImpl(
+ *backing_store, start + delete_count, *new_elms, KindTraits::Kind,
+ start + add_count, kPackedSizeNotKnown,
+ ElementsAccessor::kCopyToEndAndInitializeToHole);
+ receiver->set_elements(*new_elms);
+ return new_elms;
+ }
+
+ static Handle<Object> RemoveElement(Handle<JSArray> receiver,
+ Handle<FixedArrayBase> backing_store,
+ Where remove_position) {
+ uint32_t length =
+ static_cast<uint32_t>(Smi::cast(receiver->length())->value());
+ Isolate* isolate = receiver->GetIsolate();
+ DCHECK(length > 0);
+ int new_length = length - 1;
+ int remove_index = remove_position == AT_START ? 0 : new_length;
+ Handle<Object> result =
+ FastElementsAccessorSubclass::GetImpl(backing_store, remove_index);
+ if (remove_position == AT_START) {
+ Heap* heap = isolate->heap();
+ FastElementsAccessorSubclass::MoveElements(heap, backing_store, 0, 1,
+ new_length, 0, 0);
+ }
+ FastElementsAccessorSubclass::SetLengthImpl(receiver, new_length,
+ backing_store);
+
+ if (IsHoleyElementsKind(KindTraits::Kind) && result->IsTheHole()) {
+ return receiver->GetIsolate()->factory()->undefined_value();
}
- return false;
+ return result;
}
+ static uint32_t AddArguments(Handle<JSArray> receiver,
+ Handle<FixedArrayBase> backing_store,
+ Arguments* args, uint32_t add_size,
+ Where remove_position) {
+ uint32_t length = Smi::cast(receiver->length())->value();
+ DCHECK(add_size > 0);
+ uint32_t elms_len = backing_store->length();
+ // Check we do not overflow the new_length.
+ DCHECK(add_size <= static_cast<uint32_t>(Smi::kMaxValue - length));
+ uint32_t new_length = length + add_size;
- static bool SpliceGrowStep(Handle<JSArray> receiver,
- Handle<FixedArrayBase>& backing_store,
- Isolate* isolate, Heap* heap, uint32_t start,
- uint32_t delete_count, uint32_t add_count,
- uint32_t len, uint32_t new_length) {
- // Currently fixed arrays cannot grow too big, so
- // we should never hit this case.
- DCHECK((add_count - delete_count) <= (Smi::kMaxValue - len));
- // Check if backing_store needs to grow.
- if (new_length > static_cast<uint32_t>(backing_store->length())) {
+ if (new_length > elms_len) {
// New backing storage is needed.
- int capacity = new_length + (new_length >> 1) + 16;
- // partially copy all elements up to start
- Handle<FixedArrayBase> new_elms =
- FastElementsAccessorSubclass::ConvertElementsWithCapacity(
- receiver, backing_store, KindTraits::Kind, capacity, start);
- // Copy the trailing elements after start + delete_count
- FastElementsAccessorSubclass::CopyElementsImpl(
- *backing_store, start + delete_count, *new_elms, KindTraits::Kind,
- start + add_count, kPackedSizeNotKnown,
- ElementsAccessor::kCopyToEndAndInitializeToHole);
+ uint32_t capacity = JSObject::NewElementsCapacity(new_length);
+ // If we add arguments to the start we have to shift the existing objects.
+ int copy_dst_index = remove_position == AT_START ? add_size : 0;
+ // Copy over all objects to a new backing_store.
+ backing_store = FastElementsAccessorSubclass::ConvertElementsWithCapacity(
+ receiver, backing_store, KindTraits::Kind, capacity, 0,
+ copy_dst_index, ElementsAccessor::kCopyToEndAndInitializeToHole);
+ receiver->set_elements(*backing_store);
+ } else if (remove_position == AT_START) {
+ // If the backing store has enough capacity and we add elements to the
+ // start we have to shift the existing objects.
+ Isolate* isolate = receiver->GetIsolate();
+ FastElementsAccessorSubclass::MoveElements(isolate->heap(), backing_store,
+ add_size, 0, length, 0, 0);
+ }
- backing_store = new_elms;
- return true;
- } else {
- DisallowHeapAllocation no_gc;
- FastElementsAccessorSubclass::MoveElements(
- heap, backing_store, start + add_count, start + delete_count,
- (len - delete_count - start), 0, 0);
+ int insertion_index = remove_position == AT_START ? 0 : length;
+ // Copy the arguments to the start.
+ FastElementsAccessorSubclass::CopyArguments(args, backing_store, add_size,
+ 1, insertion_index);
+ // Set the length.
+ receiver->set_length(Smi::FromInt(new_length));
+ return new_length;
+ }
+
+ static void CopyArguments(Arguments* args, Handle<FixedArrayBase> dst_store,
+ uint32_t copy_size, uint32_t src_index,
+ uint32_t dst_index) {
+ // Add the provided values.
+ DisallowHeapAllocation no_gc;
+ FixedArrayBase* raw_backing_store = *dst_store;
+ WriteBarrierMode mode = raw_backing_store->GetWriteBarrierMode(no_gc);
+ for (uint32_t i = 0; i < copy_size; i++) {
+ Object* argument = (*args)[i + src_index];
+ FastElementsAccessorSubclass::SetImpl(raw_backing_store, i + dst_index,
+ argument, mode);
}
- return false;
}
};
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698