Array Builtin Refactoring: Creating API methods on ElementsAccessor
BUG=
Review URL: https://codereview.chromium.org/1260283002
Cr-Commit-Position: refs/heads/master@{#29958}
diff --git a/src/builtins.cc b/src/builtins.cc
index 9e04aaa..42ef5ec 100644
--- a/src/builtins.cc
+++ b/src/builtins.cc
@@ -262,7 +262,7 @@
ElementsKind target_kind = origin_kind;
{
DisallowHeapAllocation no_gc;
- int arg_count = args->length() - first_added_arg;
+ int arg_count = args_length - first_added_arg;
Object** arguments = args->arguments() - first_added_arg - (arg_count - 1);
for (int i = 0; i < arg_count; i++) {
Object* arg = arguments[i];
@@ -321,104 +321,22 @@
if (!maybe_elms_obj.ToHandle(&elms_obj)) {
return CallJsBuiltin(isolate, "$arrayPush", args);
}
-
+ // Fast Elements Path
+ int push_size = args.length() - 1;
Handle<JSArray> array = Handle<JSArray>::cast(receiver);
int len = Smi::cast(array->length())->value();
- int to_add = args.length() - 1;
- if (to_add > 0 && JSArray::WouldChangeReadOnlyLength(array, len + to_add)) {
+ if (push_size == 0) {
+ return Smi::FromInt(len);
+ }
+ if (push_size > 0 &&
+ JSArray::WouldChangeReadOnlyLength(array, len + push_size)) {
return CallJsBuiltin(isolate, "$arrayPush", args);
}
DCHECK(!array->map()->is_observed());
-
- ElementsKind kind = array->GetElementsKind();
-
- if (IsFastSmiOrObjectElementsKind(kind)) {
- Handle<FixedArray> elms = Handle<FixedArray>::cast(elms_obj);
- if (to_add == 0) {
- return Smi::FromInt(len);
- }
- // Currently fixed arrays cannot grow too big, so
- // we should never hit this case.
- DCHECK(to_add <= (Smi::kMaxValue - len));
-
- int new_length = len + to_add;
-
- 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);
-
- ElementsAccessor* accessor = array->GetElementsAccessor();
- accessor->CopyElements(
- elms_obj, 0, kind, new_elms, 0,
- ElementsAccessor::kCopyToEndAndInitializeToHole);
-
- elms = new_elms;
- }
-
- // Add the provided values.
- DisallowHeapAllocation no_gc;
- WriteBarrierMode mode = elms->GetWriteBarrierMode(no_gc);
- for (int index = 0; index < to_add; index++) {
- elms->set(index + len, args[index + 1], mode);
- }
-
- if (*elms != array->elements()) {
- array->set_elements(*elms);
- }
-
- // Set the length.
- array->set_length(Smi::FromInt(new_length));
- return Smi::FromInt(new_length);
- } else {
- int elms_len = elms_obj->length();
- if (to_add == 0) {
- return Smi::FromInt(len);
- }
- // Currently fixed arrays cannot grow too big, so
- // we should never hit this case.
- DCHECK(to_add <= (Smi::kMaxValue - len));
-
- int new_length = len + to_add;
-
- Handle<FixedDoubleArray> new_elms;
-
- if (new_length > elms_len) {
- // New backing storage is needed.
- int capacity = new_length + (new_length >> 1) + 16;
- // Create new backing store; since capacity > 0, we can
- // safely cast to FixedDoubleArray.
- new_elms = Handle<FixedDoubleArray>::cast(
- isolate->factory()->NewFixedDoubleArray(capacity));
-
- ElementsAccessor* accessor = array->GetElementsAccessor();
- accessor->CopyElements(
- elms_obj, 0, kind, new_elms, 0,
- ElementsAccessor::kCopyToEndAndInitializeToHole);
-
- } else {
- // to_add is > 0 and new_length <= elms_len, so elms_obj cannot be the
- // empty_fixed_array.
- new_elms = Handle<FixedDoubleArray>::cast(elms_obj);
- }
-
- // Add the provided values.
- DisallowHeapAllocation no_gc;
- int index;
- for (index = 0; index < to_add; index++) {
- Object* arg = args[index + 1];
- new_elms->set(index + len, arg->Number());
- }
-
- if (*new_elms != array->elements()) {
- array->set_elements(*new_elms);
- }
-
- // Set the length.
- array->set_length(Smi::FromInt(new_length));
- return Smi::FromInt(new_length);
- }
+ ElementsAccessor* accessor = array->GetElementsAccessor();
+ int new_length = accessor->Push(array, elms_obj, &args[1], push_size,
+ ElementsAccessor::kDirectionReverse);
+ return Smi::FromInt(new_length);
}
@@ -503,7 +421,6 @@
BUILTIN(ArrayUnshift) {
HandleScope scope(isolate);
- Heap* heap = isolate->heap();
Handle<Object> receiver = args.receiver();
MaybeHandle<FixedArrayBase> maybe_elms_obj =
EnsureJSArrayWithWritableFastElements(isolate, receiver, &args, 1);
@@ -545,6 +462,7 @@
array->set_elements(*elms);
} else {
DisallowHeapAllocation no_gc;
+ Heap* heap = isolate->heap();
heap->MoveElements(*elms, to_add, 0, len);
}
diff --git a/src/elements.cc b/src/elements.cc
index 7ef13b3..76269eb 100644
--- a/src/elements.cc
+++ b/src/elements.cc
@@ -245,6 +245,7 @@
}
}
}
+
DCHECK((copy_size + static_cast<int>(to_start)) <= to_base->length() &&
(copy_size + static_cast<int>(from_start)) <= from_base->length());
if (copy_size == 0) return;
@@ -571,6 +572,20 @@
UNREACHABLE();
}
+ virtual uint32_t Push(Handle<JSArray> receiver,
+ Handle<FixedArrayBase> backing_store, Object** objects,
+ uint32_t push_size, int direction) {
+ return ElementsAccessorSubclass::PushImpl(receiver, backing_store, objects,
+ push_size, direction);
+ }
+
+ static uint32_t PushImpl(Handle<JSArray> receiver,
+ Handle<FixedArrayBase> elms_obj, Object** objects,
+ uint32_t push_size, int direction) {
+ UNREACHABLE();
+ return 0;
+ }
+
virtual void SetLength(Handle<JSArray> array, uint32_t length) final {
ElementsAccessorSubclass::SetLengthImpl(array, length,
handle(array->elements()));
@@ -1140,6 +1155,53 @@
}
#endif
}
+
+ static uint32_t PushImpl(Handle<JSArray> receiver,
+ Handle<FixedArrayBase> backing_store,
+ Object** objects, uint32_t push_size,
+ int direction) {
+ uint32_t len = Smi::cast(receiver->length())->value();
+ if (push_size == 0) {
+ return len;
+ }
+ 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;
+ Handle<FixedArrayBase> new_elms;
+
+ if (new_length > elms_len) {
+ // New backing storage is needed.
+ uint32_t capacity = new_length + (new_length >> 1) + 16;
+ new_elms = FastElementsAccessorSubclass::ConvertElementsWithCapacity(
+ receiver, backing_store, KindTraits::Kind, capacity);
+ } else {
+ // push_size is > 0 and new_length <= elms_len, so backing_store cannot be
+ // the
+ // empty_fixed_array.
+ new_elms = backing_store;
+ }
+
+ // Add the provided values.
+ DisallowHeapAllocation no_gc;
+ DCHECK(direction == ElementsAccessor::kDirectionForward ||
+ direction == ElementsAccessor::kDirectionReverse);
+ STATIC_ASSERT(ElementsAccessor::kDirectionForward == 1);
+ STATIC_ASSERT(ElementsAccessor::kDirectionReverse == -1);
+ for (uint32_t index = 0; index < push_size; index++) {
+ int offset = direction * index;
+ Object* object = objects[offset];
+ FastElementsAccessorSubclass::SetImpl(*new_elms, index + len, object);
+ }
+ if (!new_elms.is_identical_to(backing_store)) {
+ receiver->set_elements(*new_elms);
+ }
+ DCHECK(*new_elms == receiver->elements());
+ // Set the length.
+ receiver->set_length(Smi::FromInt(new_length));
+ return new_length;
+ }
};
diff --git a/src/elements.h b/src/elements.h
index 511b636..0131f0b 100644
--- a/src/elements.h
+++ b/src/elements.h
@@ -60,6 +60,9 @@
// destination array with the hole.
static const int kCopyToEndAndInitializeToHole = -2;
+ static const int kDirectionForward = 1;
+ static const int kDirectionReverse = -1;
+
// Copy elements from one backing store to another. Typically, callers specify
// the source JSObject or JSArray in source_holder. If the holder's backing
// store is available, it can be passed in source and source_holder is
@@ -112,14 +115,22 @@
virtual void Set(FixedArrayBase* backing_store, uint32_t entry,
Object* value) = 0;
+
virtual void Reconfigure(Handle<JSObject> object,
Handle<FixedArrayBase> backing_store, uint32_t entry,
Handle<Object> value,
PropertyAttributes attributes) = 0;
+
virtual void Add(Handle<JSObject> object, uint32_t index,
Handle<Object> value, PropertyAttributes attributes,
uint32_t new_capacity) = 0;
+ // TODO(cbruni): Consider passing Arguments* instead of Object** depending on
+ // the requirements of future callers.
+ virtual uint32_t Push(Handle<JSArray> receiver,
+ Handle<FixedArrayBase> backing_store, Object** objects,
+ uint32_t start, int direction) = 0;
+
protected:
friend class LookupIterator;
diff --git a/test/mjsunit/array-functions-prototype-misc.js b/test/mjsunit/array-functions-prototype-misc.js
index 74dc9a6..a2c1410 100644
--- a/test/mjsunit/array-functions-prototype-misc.js
+++ b/test/mjsunit/array-functions-prototype-misc.js
@@ -312,3 +312,75 @@
// Test http://code.google.com/p/chromium/issues/detail?id=21860
Array.prototype.push.apply([], [1].splice(0, -(-1 % 5)));
+
+
+// Check that the Array functions work also properly on non-Arrays
+var receiver;
+
+receiver = 'a string';
+assertThrows(function(){
+ Array.prototype.push.call(receiver);
+});
+
+receiver = 0;
+assertEquals(undefined, receiver.length);
+assertEquals(0, Array.prototype.push.call(receiver));
+assertEquals(1, Array.prototype.push.call(receiver, 'first'));
+assertEquals(undefined, receiver.length);
+
+receiver = {};
+assertEquals(undefined, receiver.length);
+assertEquals(0, Array.prototype.push.call(receiver));
+assertEquals(0, Array.prototype.push.call(receiver));
+assertEquals(0, receiver.length);
+assertEquals(1, Array.prototype.push.call(receiver, 'first'));
+assertEquals(1, receiver.length);
+assertEquals('first', receiver[0]);
+assertEquals(2, Array.prototype.push.call(receiver, 'second'));
+assertEquals(2, receiver.length);
+assertEquals('first', receiver[0]);
+assertEquals('second', receiver[1]);
+
+receiver = {'length': 10};
+assertEquals(10, Array.prototype.push.call(receiver));
+assertEquals(10, receiver.length);
+assertEquals(11, Array.prototype.push.call(receiver, 'first'));
+assertEquals(11, receiver.length);
+assertEquals('first', receiver[10]);
+assertEquals(13, Array.prototype.push.call(receiver, 'second', 'third'));
+assertEquals(13, receiver.length);
+assertEquals('first', receiver[10]);
+assertEquals('second', receiver[11]);
+assertEquals('third', receiver[12]);
+
+receiver = {
+ get length() { return 10; },
+ set length(l) {}
+};
+assertEquals(10, Array.prototype.push.call(receiver));
+assertEquals(10, receiver.length);
+assertEquals(11, Array.prototype.push.call(receiver, 'first'));
+assertEquals(10, receiver.length);
+assertEquals('first', receiver[10]);
+assertEquals(12, Array.prototype.push.call(receiver, 'second', 'third'));
+assertEquals(10, receiver.length);
+assertEquals('second', receiver[10]);
+assertEquals('third', receiver[11]);
+
+// readonly length
+receiver = {
+ get length() { return 10; },
+};
+assertThrows(function(){
+ Array.prototype.push.call(receiver);
+});
+
+receiver = {
+ set length(l) {}
+};
+assertEquals(0, Array.prototype.push.call(receiver));
+assertEquals(undefined, receiver.length);
+assertEquals(1, Array.prototype.push.call(receiver, 'first'));
+assertEquals(undefined, receiver.length);
+assertEquals(2, Array.prototype.push.call(receiver, 'third', 'second'));
+assertEquals(undefined, receiver.length);
