[runtime] Speed up C++ version of ArrayPush

src/builtins.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/builtins.h"
 
 #include "src/api.h"
 #include "src/api-natives.h"
 #include "src/arguments.h"
 #include "src/base/once.h"
@@ skipping 227 matching lines @@
       isolate->is_initial_array_prototype(JSArray::cast(prototype)) &&
       isolate->IsFastArrayConstructorPrototypeChainIntact()) {
     return true;
   }
 
   // Slow case.
   PrototypeIterator iter(isolate, receiver);
   return PrototypeHasNoElements(&iter);
 }
 
-
-// Returns empty handle if not applicable.
+// Returns |false| if not applicable.
 MUST_USE_RESULT
-inline MaybeHandle<FixedArrayBase> EnsureJSArrayWithWritableFastElements(
-    Isolate* isolate, Handle<Object> receiver, Arguments* args,
-    int first_added_arg) {
-  // We explicitly add a HandleScope to avoid creating several copies of the
-  // same handle which would otherwise cause issue when left-trimming later-on.
-  HandleScope scope(isolate);
-  if (!receiver->IsJSArray()) return MaybeHandle<FixedArrayBase>();
+inline bool EnsureJSArrayWithWritableFastElements(Isolate* isolate,
+                                                  Handle<Object> receiver,
+                                                  Arguments* args,
+                                                  int first_added_arg) {
+  if (!receiver->IsJSArray()) return false;
   Handle<JSArray> array = Handle<JSArray>::cast(receiver);
   // If there may be elements accessors in the prototype chain, the fast path
   // cannot be used if there arguments to add to the array.
   Heap* heap = isolate->heap();
   if (args != NULL && !IsJSArrayFastElementMovingAllowed(isolate, *array)) {
-    return MaybeHandle<FixedArrayBase>();
+    return false;
   }
-  if (array->map()->is_observed()) return MaybeHandle<FixedArrayBase>();
-  if (!array->map()->is_extensible()) return MaybeHandle<FixedArrayBase>();
-  Handle<FixedArrayBase> elms(array->elements(), isolate);
-  Map* map = elms->map();
+  if (array->map()->is_observed()) return false;
+  if (!array->map()->is_extensible()) return false;
+  Map* map = array->elements()->map();
   if (map == heap->fixed_array_map()) {
     if (args == NULL || array->HasFastObjectElements()) {
-      return scope.CloseAndEscape(elms);
+      return true;
     }
   } else if (map == heap->fixed_cow_array_map()) {
-    elms = JSObject::EnsureWritableFastElements(array);
+    // Use a short-lived HandleScope to avoid creating several copies of the

[Toon Verwaest, 2016/02/24 10:31:05]

Push shouldn't need to EnsureWritableFastElements since there wouldn't be space
in the COW array to add the value anyway. It would need to copy anyway. It seems
like EnsureWritableFastElement should only be done in the ElementsAccessor in
the cases where it matters; and probably this entire method should be handled by
the ElementsAccessors so it can be compiled away where irrelevant.

+    // elements handle which would cause issues when left-trimming later-on.
+    HandleScope scope(isolate);
+    JSObject::EnsureWritableFastElements(array);
     if (args == NULL || array->HasFastObjectElements()) {
-      return scope.CloseAndEscape(elms);
+      return true;
     }
   } else if (map == heap->fixed_double_array_map()) {
     if (args == NULL) {
-      return scope.CloseAndEscape(elms);
+      return true;
     }
   } else {
-    return MaybeHandle<FixedArrayBase>();
+    return false;
   }
 
   // Adding elements to the array prototype would break code that makes sure
   // it has no elements. Handle that elsewhere.
   if (isolate->IsAnyInitialArrayPrototype(array)) {
-    return MaybeHandle<FixedArrayBase>();
+    return false;
   }
 
   // Need to ensure that the arguments passed in args can be contained in
   // the array.
   int args_length = args->length();
   if (first_added_arg >= args_length) {
-    return scope.CloseAndEscape(elms);
+    return true;
   }
 
   ElementsKind origin_kind = array->map()->elements_kind();
   DCHECK(!IsFastObjectElementsKind(origin_kind));
   ElementsKind target_kind = origin_kind;
   {
     DisallowHeapAllocation no_gc;
     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];
       if (arg->IsHeapObject()) {
         if (arg->IsHeapNumber()) {
           target_kind = FAST_DOUBLE_ELEMENTS;
         } else {
           target_kind = FAST_ELEMENTS;
           break;
         }
       }
     }
   }
   if (target_kind != origin_kind) {
+    // Use a short-lived HandleScope to avoid creating several copies of the
+    // elements handle which would cause issues when left-trimming later-on.
+    HandleScope scope(isolate);
     JSObject::TransitionElementsKind(array, target_kind);
-    elms = handle(array->elements(), isolate);
   }
-  return scope.CloseAndEscape(elms);
+  return true;
 }
 
 
 MUST_USE_RESULT static Object* CallJsIntrinsic(
     Isolate* isolate, Handle<JSFunction> function,
     BuiltinArguments<BuiltinExtraArguments::kNone> args) {
   HandleScope handleScope(isolate);
   int argc = args.length() - 1;
   ScopedVector<Handle<Object> > argv(argc);
   for (int i = 0; i < argc; ++i) {
@@ skipping 19 matching lines @@
   return isolate->heap()->undefined_value();  // Make compiler happy.
 }
 
 
 BUILTIN(EmptyFunction) { return isolate->heap()->undefined_value(); }
 
 
 BUILTIN(ArrayPush) {
   HandleScope scope(isolate);
   Handle<Object> receiver = args.receiver();
-  MaybeHandle<FixedArrayBase> maybe_elms_obj =
-      EnsureJSArrayWithWritableFastElements(isolate, receiver, &args, 1);
-  Handle<FixedArrayBase> elms_obj;
-  if (!maybe_elms_obj.ToHandle(&elms_obj)) {
+  if (!EnsureJSArrayWithWritableFastElements(isolate, receiver, &args, 1)) {
     return CallJsIntrinsic(isolate, isolate->array_push(), args);
   }
   // Fast Elements Path
   int push_size = args.length() - 1;
   Handle<JSArray> array = Handle<JSArray>::cast(receiver);
   int len = Smi::cast(array->length())->value();
   if (push_size == 0) {
     return Smi::FromInt(len);
   }
-  if (push_size > 0 &&
-      JSArray::WouldChangeReadOnlyLength(array, len + push_size)) {
+  DCHECK(push_size > 0);
+  if (JSArray::HasReadOnlyLength(array)) {
     return CallJsIntrinsic(isolate, isolate->array_push(), args);
   }
   DCHECK(!array->map()->is_observed());
   ElementsAccessor* accessor = array->GetElementsAccessor();
-  int new_length = accessor->Push(array, elms_obj, &args, push_size);
+  int new_length = accessor->Push(array, handle(array->elements(), isolate),
+                                  &args, push_size);
   return Smi::FromInt(new_length);
 }
 
 
 BUILTIN(ArrayPop) {
   HandleScope scope(isolate);
   Handle<Object> receiver = args.receiver();
-  MaybeHandle<FixedArrayBase> maybe_elms_obj =
-      EnsureJSArrayWithWritableFastElements(isolate, receiver, NULL, 0);
-  Handle<FixedArrayBase> elms_obj;
-  if (!maybe_elms_obj.ToHandle(&elms_obj)) {
+  if (!EnsureJSArrayWithWritableFastElements(isolate, receiver, NULL, 0)) {
     return CallJsIntrinsic(isolate, isolate->array_pop(), args);
   }
 
   Handle<JSArray> array = Handle<JSArray>::cast(receiver);
   DCHECK(!array->map()->is_observed());
 
   uint32_t len = static_cast<uint32_t>(Smi::cast(array->length())->value());
   if (len == 0) return isolate->heap()->undefined_value();
 
   if (JSArray::HasReadOnlyLength(array)) {
     return CallJsIntrinsic(isolate, isolate->array_pop(), args);
   }
 
   Handle<Object> result;
   if (IsJSArrayFastElementMovingAllowed(isolate, JSArray::cast(*receiver))) {
     // Fast Elements Path
-    result = array->GetElementsAccessor()->Pop(array, elms_obj);
+    result = array->GetElementsAccessor()->Pop(
+        array, handle(array->elements(), isolate));
   } else {
     // Use Slow Lookup otherwise
     uint32_t new_length = len - 1;
     ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
         isolate, result, Object::GetElement(isolate, array, new_length));
     JSArray::SetLength(array, new_length);
   }
   return *result;
 }
 
 
 BUILTIN(ArrayShift) {
   HandleScope scope(isolate);
   Heap* heap = isolate->heap();
   Handle<Object> receiver = args.receiver();
-  MaybeHandle<FixedArrayBase> maybe_elms_obj =
-      EnsureJSArrayWithWritableFastElements(isolate, receiver, NULL, 0);
-  Handle<FixedArrayBase> elms_obj;
-  if (!maybe_elms_obj.ToHandle(&elms_obj) ||
+  if (!EnsureJSArrayWithWritableFastElements(isolate, receiver, NULL, 0) ||
       !IsJSArrayFastElementMovingAllowed(isolate, JSArray::cast(*receiver))) {
     return CallJsIntrinsic(isolate, isolate->array_shift(), args);
   }
   Handle<JSArray> array = Handle<JSArray>::cast(receiver);
   DCHECK(!array->map()->is_observed());
 
   int len = Smi::cast(array->length())->value();
   if (len == 0) return heap->undefined_value();
 
   if (JSArray::HasReadOnlyLength(array)) {
     return CallJsIntrinsic(isolate, isolate->array_shift(), args);
   }
 
-  Handle<Object> first = array->GetElementsAccessor()->Shift(array, elms_obj);
+  Handle<Object> first = array->GetElementsAccessor()->Shift(
+      array, handle(array->elements(), isolate));
   return *first;
 }
 
 
 BUILTIN(ArrayUnshift) {
   HandleScope scope(isolate);
   Handle<Object> receiver = args.receiver();
-  MaybeHandle<FixedArrayBase> maybe_elms_obj =
-      EnsureJSArrayWithWritableFastElements(isolate, receiver, &args, 1);
-  Handle<FixedArrayBase> elms_obj;
-  if (!maybe_elms_obj.ToHandle(&elms_obj)) {
+  if (!EnsureJSArrayWithWritableFastElements(isolate, receiver, &args, 1)) {
     return CallJsIntrinsic(isolate, isolate->array_unshift(), args);
   }
   Handle<JSArray> array = Handle<JSArray>::cast(receiver);
   DCHECK(!array->map()->is_observed());
   int to_add = args.length() - 1;
   if (to_add == 0) {
     return array->length();
   }
   // Currently fixed arrays cannot grow too big, so
   // we should never hit this case.
   DCHECK(to_add <= (Smi::kMaxValue - Smi::cast(array->length())->value()));
 
   if (to_add > 0 && JSArray::HasReadOnlyLength(array)) {
     return CallJsIntrinsic(isolate, isolate->array_unshift(), args);
   }
 
   ElementsAccessor* accessor = array->GetElementsAccessor();
-  int new_length = accessor->Unshift(array, elms_obj, &args, to_add);
+  int new_length = accessor->Unshift(array, handle(array->elements(), isolate),
+                                     &args, to_add);
   return Smi::FromInt(new_length);
 }
 
 
 BUILTIN(ArraySlice) {
   HandleScope scope(isolate);
   Handle<Object> receiver = args.receiver();
   Handle<JSObject> object;
   Handle<FixedArrayBase> elms_obj;
   int len = -1;
@@ skipping 81 matching lines @@
   }
   Handle<JSArray> result_array =
       accessor->Slice(object, elms_obj, actual_start, actual_end);
   return *result_array;
 }
 
 
 BUILTIN(ArraySplice) {
   HandleScope scope(isolate);
   Handle<Object> receiver = args.receiver();
-  MaybeHandle<FixedArrayBase> maybe_elms_obj =
-      EnsureJSArrayWithWritableFastElements(isolate, receiver, &args, 3);
-  Handle<FixedArrayBase> elms_obj;
-  if (!maybe_elms_obj.ToHandle(&elms_obj)) {
+  if (!EnsureJSArrayWithWritableFastElements(isolate, receiver, &args, 3)) {
     return CallJsIntrinsic(isolate, isolate->array_splice(), args);
   }
   // TODO(littledan): Look up @@species only once, not once here and
   // again in the JS builtin. Pass the species out?
   Handle<Object> species;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
       isolate, species, Object::ArraySpeciesConstructor(isolate, receiver));
   if (*species != isolate->context()->native_context()->array_function()) {
     return CallJsIntrinsic(isolate, isolate->array_splice(), args);
   }
@@ skipping 35 matching lines @@
   }
 
   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 CallJsIntrinsic(isolate, isolate->array_splice(), args);
   }
   ElementsAccessor* accessor = array->GetElementsAccessor();
-  Handle<JSArray> result_array = accessor->Splice(
-      array, elms_obj, actual_start, actual_delete_count, &args, add_count);
+  Handle<JSArray> result_array =
+      accessor->Splice(array, handle(array->elements(), isolate), actual_start,
+                       actual_delete_count, &args, add_count);
   return *result_array;
 }
 
 
 // Array Concat -------------------------------------------------------------
 
 namespace {
 
 /**
  * A simple visitor visits every element of Array's.
@@ skipping 3803 matching lines @@
 BUILTIN_LIST_C(DEFINE_BUILTIN_ACCESSOR_C)
 BUILTIN_LIST_A(DEFINE_BUILTIN_ACCESSOR_A)
 BUILTIN_LIST_H(DEFINE_BUILTIN_ACCESSOR_H)
 BUILTIN_LIST_DEBUG_A(DEFINE_BUILTIN_ACCESSOR_A)
 #undef DEFINE_BUILTIN_ACCESSOR_C
 #undef DEFINE_BUILTIN_ACCESSOR_A
 
 
 }  // namespace internal
 }  // namespace v8