Skip to content

V
V8
Commit 8533d4b5 authored by cbruni's avatar cbruni Committed by Commit bot
Browse files
Options

- remove the Backing-Store speficic code from builtins.cc and put it in elements.cc. 

- adding tests to improve coverage of the splice method

BUG=

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

Cr-Commit-Position: refs/heads/master@{#30269}
parent 4e39437a
Expand all Show whitespace changes
Inline Side-by-side
Showing with 285 additions and 203 deletions
src/builtins.cc
View file @ 8533d4b5
......@@ -176,6 +176,27 @@ BUILTIN(EmptyFunction) {
}
// TODO(cbruni): check if this is a suitable method on Object
bool ClampedToInteger(Object* object, int* out) {
// This is an extended version of ECMA-262 9.4, but additionally
// clamps values to [kMinInt, kMaxInt]
if (object->IsSmi()) {
*out = Smi::cast(object)->value();
return true;
} else if (object->IsHeapNumber()) {
*out = FastD2IChecked(HeapNumber::cast(object)->value());
return true;
} else if (object->IsUndefined()) {
*out = 0;
return true;
} else if (object->IsBoolean()) {
*out = (Oddball::cast(object)->kind() == Oddball::kTrue) ? 1 : 0;
return true;
}
return false;
}
static void MoveDoubleElements(FixedDoubleArray* dst, int dst_index,
FixedDoubleArray* src, int src_index, int len) {
if (len == 0) return;
......@@ -621,7 +642,6 @@ BUILTIN(ArraySlice) {
BUILTIN(ArraySplice) {
HandleScope scope(isolate);
Heap* heap = isolate->heap();
Handle<Object> receiver = args.receiver();
MaybeHandle<FixedArrayBase> maybe_elms_obj =
EnsureJSArrayWithWritableFastElements(isolate, receiver, &args, 3);
......@@ -632,209 +652,51 @@ BUILTIN(ArraySplice) {
Handle<JSArray> array = Handle<JSArray>::cast(receiver);
DCHECK(!array->map()->is_observed());
int len = Smi::cast(array->length())->value();
int n_arguments = args.length() - 1;
int argument_count = args.length() - 1;
int relative_start = 0;
if (n_arguments > 0) {
if (argument_count > 0) {
DisallowHeapAllocation no_gc;
Object* arg1 = args[1];
if (arg1->IsSmi()) {
relative_start = Smi::cast(arg1)->value();
} else if (arg1->IsHeapNumber()) {
double start = HeapNumber::cast(arg1)->value();
if (start < kMinInt || start > kMaxInt) {
AllowHeapAllocation allow_allocation;
return CallJsBuiltin(isolate, "$arraySplice", args);
}
relative_start = std::isnan(start) ? 0 : static_cast<int>(start);
} else if (!arg1->IsUndefined()) {
if (!ClampedToInteger(args[1], &relative_start)) {
AllowHeapAllocation allow_allocation;
return CallJsBuiltin(isolate, "$arraySplice", args);
}
}
int len = Smi::cast(array->length())->value();
// clip relative start to [0, len]
int actual_start = (relative_start < 0) ? Max(len + relative_start, 0)
: Min(relative_start, len);
int actual_delete_count;
if (argument_count == 1) {
// SpiderMonkey, TraceMonkey and JSC treat the case where no delete count is
// given as a request to delete all the elements from the start.
// And it differs from the case of undefined delete count.
// This does not follow ECMA-262, but we do the same for
// compatibility.
int actual_delete_count;
if (n_arguments == 1) {
// This does not follow ECMA-262, but we do the same for compatibility.
DCHECK(len - actual_start >= 0);
actual_delete_count = len - actual_start;
} else {
int value = 0; // ToInteger(undefined) == 0
if (n_arguments > 1) {
int delete_count = 0;
DisallowHeapAllocation no_gc;
Object* arg2 = args[2];
if (arg2->IsSmi()) {
value = Smi::cast(arg2)->value();
} else {
if (argument_count > 1) {
if (!ClampedToInteger(args[2], &delete_count)) {
AllowHeapAllocation allow_allocation;
return CallJsBuiltin(isolate, "$arraySplice", args);
}
}
actual_delete_count = Min(Max(value, 0), len - actual_start);
actual_delete_count = Min(Max(delete_count, 0), len - actual_start);
}
ElementsKind elements_kind = array->GetElementsKind();
int item_count = (n_arguments > 1) ? (n_arguments - 2) : 0;
int new_length = len - actual_delete_count + item_count;
// For double mode we do not support changing the length.
if (new_length > len && IsFastDoubleElementsKind(elements_kind)) {
return CallJsBuiltin(isolate, "$arraySplice", args);
}
int add_count = (argument_count > 1) ? (argument_count - 2) : 0;
int new_length = len - actual_delete_count + add_count;
if (new_length != len && JSArray::HasReadOnlyLength(array)) {
AllowHeapAllocation allow_allocation;
return CallJsBuiltin(isolate, "$arraySplice", args);
}
if (new_length == 0) {
Handle<JSArray> result = isolate->factory()->NewJSArrayWithElements(
elms_obj, elements_kind, actual_delete_count);
array->set_elements(heap->empty_fixed_array());
array->set_length(Smi::FromInt(0));
return *result;
}
Handle<JSArray> result_array =
isolate->factory()->NewJSArray(elements_kind,
actual_delete_count,
actual_delete_count);
if (actual_delete_count > 0) {
DisallowHeapAllocation no_gc;
ElementsAccessor* accessor = array->GetElementsAccessor();
accessor->CopyElements(
elms_obj, actual_start, elements_kind,
handle(result_array->elements(), isolate), 0, actual_delete_count);
}
bool elms_changed = false;
if (item_count < actual_delete_count) {
// Shrink the array.
const bool trim_array = !heap->lo_space()->Contains(*elms_obj) &&
((actual_start + item_count) <
(len - actual_delete_count - actual_start));
if (trim_array) {
const int delta = actual_delete_count - item_count;
if (elms_obj->IsFixedDoubleArray()) {
Handle<FixedDoubleArray> elms =
Handle<FixedDoubleArray>::cast(elms_obj);
MoveDoubleElements(*elms, delta, *elms, 0, actual_start);
} else {
Handle<FixedArray> elms = Handle<FixedArray>::cast(elms_obj);
DisallowHeapAllocation no_gc;
heap->MoveElements(*elms, delta, 0, actual_start);
}
if (heap->CanMoveObjectStart(*elms_obj)) {
// On the fast path we move the start of the object in memory.
elms_obj = handle(heap->LeftTrimFixedArray(*elms_obj, delta));
} else {
// This is the slow path. We are going to move the elements to the left
// by copying them. For trimmed values we store the hole.
if (elms_obj->IsFixedDoubleArray()) {
Handle<FixedDoubleArray> elms =
Handle<FixedDoubleArray>::cast(elms_obj);
MoveDoubleElements(*elms, 0, *elms, delta, len - delta);
elms->FillWithHoles(len - delta, len);
} else {
Handle<FixedArray> elms = Handle<FixedArray>::cast(elms_obj);
DisallowHeapAllocation no_gc;
heap->MoveElements(*elms, 0, delta, len - delta);
elms->FillWithHoles(len - delta, len);
}
}
elms_changed = true;
} else {
if (elms_obj->IsFixedDoubleArray()) {
Handle<FixedDoubleArray> elms =
Handle<FixedDoubleArray>::cast(elms_obj);
MoveDoubleElements(*elms, actual_start + item_count,
*elms, actual_start + actual_delete_count,
(len - actual_delete_count - actual_start));
elms->FillWithHoles(new_length, len);
} else {
Handle<FixedArray> elms = Handle<FixedArray>::cast(elms_obj);
DisallowHeapAllocation no_gc;
heap->MoveElements(*elms, actual_start + item_count,
actual_start + actual_delete_count,
(len - actual_delete_count - actual_start));
elms->FillWithHoles(new_length, len);
}
}
} else if (item_count > actual_delete_count) {
Handle<FixedArray> elms = Handle<FixedArray>::cast(elms_obj);
// Currently fixed arrays cannot grow too big, so
// we should never hit this case.
DCHECK((item_count - actual_delete_count) <= (Smi::kMaxValue - len));
// Check if array need to grow.
if (new_length > elms->length()) {
// New backing storage is needed.
int capacity = new_length + (new_length >> 1) + 16;
Handle<FixedArray> new_elms =
isolate->factory()->NewUninitializedFixedArray(capacity);
DisallowHeapAllocation no_gc;
ElementsKind kind = array->GetElementsKind();
ElementsAccessor* accessor = array->GetElementsAccessor();
if (actual_start > 0) {
// Copy the part before actual_start as is.
accessor->CopyElements(
elms, 0, kind, new_elms, 0, actual_start);
}
accessor->CopyElements(
elms, actual_start + actual_delete_count, kind,
new_elms, actual_start + item_count,
ElementsAccessor::kCopyToEndAndInitializeToHole);
elms_obj = new_elms;
elms_changed = true;
} else {
DisallowHeapAllocation no_gc;
heap->MoveElements(*elms, actual_start + item_count,
actual_start + actual_delete_count,
(len - actual_delete_count - actual_start));
}
}
if (IsFastDoubleElementsKind(elements_kind)) {
Handle<FixedDoubleArray> elms = Handle<FixedDoubleArray>::cast(elms_obj);
for (int k = actual_start; k < actual_start + item_count; k++) {
Object* arg = args[3 + k - actual_start];
if (arg->IsSmi()) {
elms->set(k, Smi::cast(arg)->value());
} else {
elms->set(k, HeapNumber::cast(arg)->value());
}
}
} else {
Handle<FixedArray> elms = Handle<FixedArray>::cast(elms_obj);
DisallowHeapAllocation no_gc;
WriteBarrierMode mode = elms->GetWriteBarrierMode(no_gc);
for (int k = actual_start; k < actual_start + item_count; k++) {
elms->set(k, args[3 + k - actual_start], mode);
}
}
if (elms_changed) {
array->set_elements(*elms_obj);
}
// Set the length.
array->set_length(Smi::FromInt(new_length));
return *result_array;
Handle<JSArray> result = accessor->Splice(
array, elms_obj, actual_start, actual_delete_count, args, add_count);
return *result;
}
......
src/elements.cc
View file @ 8533d4b5
This diff is collapsed.
src/elements.h
View file @ 8533d4b5
......@@ -131,6 +131,11 @@ class ElementsAccessor {
Handle<FixedArrayBase> backing_store, Object** objects,
uint32_t start, int direction) = 0;
virtual Handle<JSArray> Splice(Handle<JSArray> receiver,
Handle<FixedArrayBase> backing_store,
uint32_t start, uint32_t delete_count,
Arguments args, uint32_t add_count) = 0;
protected:
friend class LookupIterator;
......
test/mjsunit/array-natives-elements.js
View file @ 8533d4b5
......@@ -151,7 +151,6 @@ function array_natives_test() {
assertTrue(%HasFastSmiElements(a3));
assertEquals([1], a3r);
assertEquals([2, 2, 3], a3);
a3 = [1.1,2,3];
a3r = a3.splice(0, 0);
assertTrue(%HasFastDoubleElements(a3r));
......@@ -166,13 +165,12 @@ function array_natives_test() {
assertEquals([2, 3], a3);
a3 = [1.1,2,3];
a3r = a3.splice(0, 0, 2);
// Commented out since handled in js, which takes the best fit.
// assertTrue(%HasFastDoubleElements(a3r));
assertTrue(%HasFastSmiElements(a3r));
assertTrue(%HasFastDoubleElements(a3r));
assertTrue(%HasFastDoubleElements(a3));
assertEquals([], a3r);
assertEquals([2, 1.1, 2, 3], a3);
a3 = [1.1,2,3];
assertTrue(%HasFastDoubleElements(a3));
a3r = a3.splice(0, 1, 2);
assertTrue(%HasFastDoubleElements(a3r));
assertTrue(%HasFastDoubleElements(a3));
......@@ -180,9 +178,7 @@ function array_natives_test() {
assertEquals([2, 2, 3], a3);
a3 = [1.1,2,3];
a3r = a3.splice(0, 0, 2.1);
// Commented out since handled in js, which takes the best fit.
// assertTrue(%HasFastDoubleElements(a3r));
assertTrue(%HasFastSmiElements(a3r));
assertTrue(%HasFastDoubleElements(a3r));
assertTrue(%HasFastDoubleElements(a3));
assertEquals([], a3r);
assertEquals([2.1, 1.1, 2, 3], a3);
......@@ -194,9 +190,7 @@ function array_natives_test() {
assertEquals([2.2, 2, 3], a3);
a3 = [1,2,3];
a3r = a3.splice(0, 0, 2.1);
// Commented out since handled in js, which takes the best fit.
// assertTrue(%HasFastDoubleElements(a3r));
assertTrue(%HasFastSmiElements(a3r));
assertTrue(%HasFastDoubleElements(a3r));
assertTrue(%HasFastDoubleElements(a3));
assertEquals([], a3r);
assertEquals([2.1, 1, 2, 3], a3);
......@@ -206,7 +200,6 @@ function array_natives_test() {
assertTrue(%HasFastDoubleElements(a3));
assertEquals([1], a3r);
assertEquals([2.2, 2, 3], a3);
a3 = [{},2,3];
a3r = a3.splice(0, 0);
assertTrue(%HasFastObjectElements(a3r));
......@@ -231,7 +224,6 @@ function array_natives_test() {
assertTrue(%HasFastObjectElements(a3));
assertEquals([1], a3r);
assertEquals([{}, 2, 3], a3);
a3 = [1.1,2,3];
a3r = a3.splice(0, 0, {});
assertTrue(%HasFastObjectElements(a3r));
......@@ -244,6 +236,35 @@ function array_natives_test() {
assertTrue(%HasFastObjectElements(a3));
assertEquals([1.1], a3r);
assertEquals([{}, 2, 3], a3);
// Splice large objects
var a3 = new Array(1024 * 1024);
a3[1024*1024-1] = 1;
var a3r;
a3r = a3.splice(-1, 1);
assertTrue(%HasFastSmiElements(a3r));
assertTrue(%HasFastSmiElements(a3));
assertEquals([1], a3r);
assertEquals(new Array(1024 * 1024 - 1), a3);
var a3 = new Array(1024 * 1024);
a3[0] = 1;
var a3r;
a3r = a3.splice(0, 1);
assertTrue(%HasFastSmiElements(a3r));
assertTrue(%HasFastSmiElements(a3));
assertEquals([1], a3r);
assertEquals(new Array(1024 * 1024 - 1), a3);
// Splice array with large enough backing store
a3 = [1.1, 2.2, 3.3];
a3r = a3.splice(2, 1);
assertTrue(%HasFastDoubleElements(a3r));
assertTrue(%HasFastDoubleElements(a3));
assertEquals([3.3], a3r);
assertEquals([1.1, 2.2], a3);
a3r = a3.splice(1, 1, 4.4, 5.5);
assertTrue(%HasFastDoubleElements(a3r));
assertTrue(%HasFastDoubleElements(a3));
assertEquals([2.2], a3r);
assertEquals([1.1, 4.4, 5.5], a3);
// Pop
var a4 = [1,2,3];
......
test/mjsunit/array-splice.js
View file @ 8533d4b5
......@@ -115,6 +115,11 @@
assertEquals([], array);
assertEquals([1, 2, 3, 4, 5, 6, 7], spliced);
array = [1, 2, 3, 4, 5, 6, 7];
spliced = array.splice(-1e100);
assertEquals([], array);
assertEquals([1, 2, 3, 4, 5, 6, 7], spliced);
array = [1, 2, 3, 4, 5, 6, 7];
spliced = array.splice(-3);
assertEquals([1, 2, 3, 4], array);
......@@ -145,11 +150,21 @@
assertEquals([1, 2, 3, 4, 5, 6, 7], array);
assertEquals([], spliced);
array = [1, 2, 3, 4, 5, 6, 7];
spliced = array.splice(1e100);
assertEquals([1, 2, 3, 4, 5, 6, 7], array);
assertEquals([], spliced);
array = [1, 2, 3, 4, 5, 6, 7];
spliced = array.splice(0, -100);
assertEquals([1, 2, 3, 4, 5, 6, 7], array);
assertEquals([], spliced);
array = [1, 2, 3, 4, 5, 6, 7];
spliced = array.splice(0, -1e100);
assertEquals([1, 2, 3, 4, 5, 6, 7], array);
assertEquals([], spliced);
array = [1, 2, 3, 4, 5, 6, 7];
spliced = array.splice(0, -3);
assertEquals([1, 2, 3, 4, 5, 6, 7], array);
......@@ -180,6 +195,11 @@
assertEquals([], array);
assertEquals([1, 2, 3, 4, 5, 6, 7], spliced);
array = [1, 2, 3, 4, 5, 6, 7];
spliced = array.splice(0, 1e100);
assertEquals([], array);
assertEquals([1, 2, 3, 4, 5, 6, 7], spliced);
// Some exotic cases.
obj = { toString: function() { throw 'Exception'; } };
......
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