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

Unified Diff: src/builtins.cc

Issue 1311343002: Array.prototype.unshift builtin improvements (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: signedness Created 5 years, 4 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 | src/elements.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/builtins.cc
diff --git a/src/builtins.cc b/src/builtins.cc
index 6fe2d2413fc94890f58ab2e0830ccb4cf4b135b8..0c32f36b0cb3c1a6b9673b69c65cd1c5b057f000 100644
--- a/src/builtins.cc
+++ b/src/builtins.cc
@@ -175,9 +175,9 @@ BUILTIN(EmptyFunction) {
return isolate->heap()->undefined_value();
}
+namespace {
-// TODO(cbruni): check if this is a suitable method on Object
-bool ClampedToInteger(Object* object, int* out) {
+bool ObjectToClampedInteger(Object* object, int* out) {
// This is an extended version of ECMA-262 9.4, but additionally
// clamps values to [kMinInt, kMaxInt]
if (object->IsSmi()) {
@@ -331,6 +331,7 @@ MUST_USE_RESULT static Object* CallJsBuiltin(
return *result;
}
+} // namespace
BUILTIN(ArrayPush) {
HandleScope scope(isolate);
@@ -432,7 +433,6 @@ BUILTIN(ArrayShift) {
}
}
- // Set the length.
array->set_length(Smi::FromInt(len - 1));
return *first;
@@ -450,51 +450,15 @@ BUILTIN(ArrayUnshift) {
}
Handle<JSArray> array = Handle<JSArray>::cast(receiver);
DCHECK(!array->map()->is_observed());
- if (!array->HasFastSmiOrObjectElements()) {
- return CallJsBuiltin(isolate, "$arrayUnshift", args);
- }
int len = Smi::cast(array->length())->value();
int to_add = args.length() - 1;
- int new_length = len + to_add;
- // Currently fixed arrays cannot grow too big, so
- // we should never hit this case.
- DCHECK(to_add <= (Smi::kMaxValue - len));
if (to_add > 0 && JSArray::WouldChangeReadOnlyLength(array, len + to_add)) {
return CallJsBuiltin(isolate, "$arrayUnshift", args);
}
- Handle<FixedArray> elms = Handle<FixedArray>::cast(elms_obj);
-
- 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);
-
- ElementsKind kind = array->GetElementsKind();
- ElementsAccessor* accessor = array->GetElementsAccessor();
- accessor->CopyElements(
- elms, 0, kind, new_elms, to_add,
- ElementsAccessor::kCopyToEndAndInitializeToHole);
-
- elms = new_elms;
- array->set_elements(*elms);
- } else {
- DisallowHeapAllocation no_gc;
- Heap* heap = isolate->heap();
- heap->MoveElements(*elms, to_add, 0, len);
- }
-
- // Add the provided values.
- DisallowHeapAllocation no_gc;
- WriteBarrierMode mode = elms->GetWriteBarrierMode(no_gc);
- for (int i = 0; i < to_add; i++) {
- elms->set(i, args[i + 1], mode);
- }
-
- // Set the length.
- array->set_length(Smi::FromInt(new_length));
+ ElementsAccessor* accessor = array->GetElementsAccessor();
+ int new_length = accessor->Unshift(array, elms_obj, args, to_add);
return Smi::FromInt(new_length);
}
@@ -656,7 +620,7 @@ BUILTIN(ArraySplice) {
int relative_start = 0;
if (argument_count > 0) {
DisallowHeapAllocation no_gc;
- if (!ClampedToInteger(args[1], &relative_start)) {
+ if (!ObjectToClampedInteger(args[1], &relative_start)) {
AllowHeapAllocation allow_allocation;
return CallJsBuiltin(isolate, "$arraySplice", args);
}
@@ -678,7 +642,7 @@ BUILTIN(ArraySplice) {
int delete_count = 0;
DisallowHeapAllocation no_gc;
if (argument_count > 1) {
- if (!ClampedToInteger(args[2], &delete_count)) {
+ if (!ObjectToClampedInteger(args[2], &delete_count)) {
AllowHeapAllocation allow_allocation;
return CallJsBuiltin(isolate, "$arraySplice", args);
}
« no previous file with comments | « no previous file | src/elements.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698