Reimplement Array.prototype.slice in CSA and C++

Previously, V8's slice was implemented in a combination of C++ and a 
Javascript fallback. The disadvantage of this approach was that the
fast-path required a call through the CEntryStub, which introduced
considerable overhead for small arrays with fast elements kinds.

Now the implementation primarily uses the CSA to generate both the
full spec-complaint implementation as well as fast paths for argument
objects and arrays with fast elements kinds. The CSA implementation
uses a C++ implementation fallback in select situations where the the
complexity of a CSA implementation would be too great and the
CEntryStub overhead is not decisive (e.g. slices of dictionary
elements arrays).

Performance results on semi-random arrays with small number of
elements (old vs. new):

smi copy: 48.7 ms vs. 12 ms
smi slice: 43.5 ms 14.8 ms
object copy: 35.5 ms 7.7 ms
object slice: 38.7 ms 8.8 ms
dictionary slice: 2398.3 ms vs. 5.4 ms
fast sloppy arguments slice: 9.6 ms vs. 7.2 ms
slow sloppy arguments slice: 28.9 ms vs. 8.5 ms

As a bonus, the new implementation is fully spec-compliant and fixes
at least one existing bug.

The design document for Array.prototype builtin rework can be found
at https://goo.gl/wFHe2n

Bug: v8:1956,v8:6601,v8:6710,v8:6978
Change-Id: Ia0155bedcf39b4577605ff754f416c2af938efb7
Reviewed-on: https://chromium-review.googlesource.com/574710
Commit-Queue: Daniel Clifford <danno@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48853}

src/bootstrapper.cc

@@ -1657,7 +1657,12 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
     SimpleInstallFunction(proto, "push", Builtins::kFastArrayPush, 1, false);
     SimpleInstallFunction(proto, "shift", Builtins::kFastArrayShift, 0, false);
     SimpleInstallFunction(proto, "unshift", Builtins::kArrayUnshift, 1, false);
-    SimpleInstallFunction(proto, "slice", Builtins::kArraySlice, 2, false);
+    if (FLAG_enable_experimental_builtins) {
+      SimpleInstallFunction(proto, "slice", Builtins::kFastArraySlice, 2,
+                            false);
+    } else {
+      SimpleInstallFunction(proto, "slice", Builtins::kArraySlice, 2, false);
+    }
     SimpleInstallFunction(proto, "splice", Builtins::kArraySplice, 2, false);
     SimpleInstallFunction(proto, "includes", Builtins::kArrayIncludes, 1,
                           false);

src/builtins/builtins-definitions.h

@@ -264,10 +264,14 @@ namespace internal {
   TFJ(FastArrayShift, SharedFunctionInfo::kDontAdaptArgumentsSentinel)         \
   /* ES6 #sec-array.prototype.slice */                                         \
   CPP(ArraySlice)                                                              \
+  TFJ(FastArraySlice, SharedFunctionInfo::kDontAdaptArgumentsSentinel)         \
   /* ES6 #sec-array.prototype.splice */                                        \
   CPP(ArraySplice)                                                             \
   /* ES6 #sec-array.prototype.unshift */                                       \
   CPP(ArrayUnshift)                                                            \
+  /* Support for Array.from and other array-copying idioms */                  \
+  TFS(CloneFastJSArray, kSource)                                               \
+  TFS(ExtractFastJSArray, kSource, kBegin, kCount)                             \
   /* ES6 #sec-array.prototype.foreach */                                       \
   TFS(ArrayForEachLoopContinuation, kReceiver, kCallbackFn, kThisArg, kArray,  \
       kObject, kInitialK, kLength, kTo)                                        \

src/code-factory.cc

@@ -334,6 +334,18 @@ Callable CodeFactory::ArrayShift(Isolate* isolate) {
                   BuiltinDescriptor(isolate));
 }
 
+// static
+Callable CodeFactory::ExtractFastJSArray(Isolate* isolate) {
+  return Callable(BUILTIN_CODE(isolate, ExtractFastJSArray),
+                  ExtractFastJSArrayDescriptor(isolate));
+}
+
+// static
+Callable CodeFactory::CloneFastJSArray(Isolate* isolate) {
+  return Callable(BUILTIN_CODE(isolate, CloneFastJSArray),
+                  CloneFastJSArrayDescriptor(isolate));
+}
+
 // static
 Callable CodeFactory::ArrayPush(Isolate* isolate) {
   return Callable(BUILTIN_CODE(isolate, ArrayPush), BuiltinDescriptor(isolate));

src/code-factory.h

@@ -91,6 +91,8 @@ class V8_EXPORT_PRIVATE CodeFactory final {
   static Callable ArrayPop(Isolate* isolate);
   static Callable ArrayPush(Isolate* isolate);
   static Callable ArrayShift(Isolate* isolate);
+  static Callable ExtractFastJSArray(Isolate* isolate);
+  static Callable CloneFastJSArray(Isolate* isolate);
   static Callable FunctionPrototypeBind(Isolate* isolate);
   static Callable TransitionElementsKind(Isolate* isolate, ElementsKind from,
                                          ElementsKind to, bool is_jsarray);

src/code-stub-assembler.h

@@ -597,6 +597,12 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   // Calling this is only valid if there's a module context in the chain.
   TNode<Context> LoadModuleContext(SloppyTNode<Context> context);
 
+  void GotoIfContextElementEqual(Node* value, Node* native_context,
+                                 int slot_index, Label* if_equal) {
+    GotoIf(WordEqual(value, LoadContextElement(native_context, slot_index)),
+           if_equal);
+  }
+
   TNode<Map> LoadJSArrayElementsMap(ElementsKind kind,
                                     SloppyTNode<Context> native_context);
   TNode<Map> LoadJSArrayElementsMap(SloppyTNode<Int32T> kind,

src/debug/debug-evaluate.cc

@@ -274,6 +274,7 @@ bool IntrinsicHasNoSideEffect(Runtime::FunctionId id) {
   V(ArraySpeciesConstructor)         \
   V(NormalizeElements)               \
   V(GetArrayKeys)                    \
+  V(TrySliceSimpleNonFastElements)   \
   V(HasComplexElements)              \
   V(EstimateNumberOfElements)        \
   /* Errors */                       \

src/elements.cc

@@ -1429,6 +1429,37 @@ class DictionaryElementsAccessor
     UNREACHABLE();
   }
 
+  static Handle<JSObject> SliceImpl(Handle<JSObject> receiver, uint32_t start,
+                                    uint32_t end) {
+    Isolate* isolate = receiver->GetIsolate();
+    uint32_t result_length = end < start ? 0u : end - start;
+
+    // Result must also be a dictionary.
+    Handle<JSArray> result_array =
+        isolate->factory()->NewJSArray(0, HOLEY_ELEMENTS);
+    JSObject::NormalizeElements(result_array);
+    result_array->set_length(Smi::FromInt(result_length));
+    Handle<SeededNumberDictionary> source_dict(
+        SeededNumberDictionary::cast(receiver->elements()));
+    int entry_count = source_dict->Capacity();
+    for (int i = 0; i < entry_count; i++) {
+      Object* key = source_dict->KeyAt(i);
+      if (!key->IsUndefined(isolate)) {
+        uint64_t key_value = NumberToInt64(key);
+        if (key_value >= start && key_value < end) {
+          Handle<SeededNumberDictionary> dest_dict(
+              SeededNumberDictionary::cast(result_array->elements()));
+          Handle<Object> value(source_dict->ValueAt(i), isolate);
+          PropertyDetails details = source_dict->DetailsAt(i);
+          PropertyAttributes attr = details.attributes();
+          AddImpl(result_array, static_cast<uint32_t>(key_value), value, attr,
+                  0);
+        }
+      }
+    }
+
+    return result_array;
+  }
 
   static void DeleteImpl(Handle<JSObject> obj, uint32_t entry) {
     Handle<SeededNumberDictionary> dict(
@@ -3741,6 +3772,29 @@ class SloppyArgumentsElementsAccessor
     }
     return Just<int64_t>(-1);
   }
+
+  static Handle<JSObject> SliceImpl(Handle<JSObject> receiver, uint32_t start,
+                                    uint32_t end) {
+    Isolate* isolate = receiver->GetIsolate();
+    uint32_t result_len = end < start ? 0u : end - start;
+    Handle<JSArray> result_array =
+        isolate->factory()->NewJSArray(HOLEY_ELEMENTS, result_len, result_len);
+    DisallowHeapAllocation no_gc;
+    FixedArray* elements = FixedArray::cast(result_array->elements());
+    FixedArray* parameters = FixedArray::cast(receiver->elements());
+    uint32_t insertion_index = 0;
+    for (uint32_t i = start; i < end; i++) {
+      uint32_t entry = GetEntryForIndexImpl(isolate, *receiver, parameters, i,
+                                            ALL_PROPERTIES);
+      if (entry != kMaxUInt32 && HasEntryImpl(isolate, parameters, entry)) {
+        elements->set(insertion_index, *GetImpl(isolate, parameters, entry));
+      } else {
+        elements->set_the_hole(isolate, insertion_index);
+      }
+      insertion_index++;
+    }
+    return result_array;
+  }
 };
 
 
@@ -3866,29 +3920,6 @@ class FastSloppyArgumentsElementsAccessor
     return Handle<FixedArray>(elements->arguments(), isolate);
   }
 
-  static Handle<JSObject> SliceImpl(Handle<JSObject> receiver, uint32_t start,
-                                    uint32_t end) {
-    Isolate* isolate = receiver->GetIsolate();
-    uint32_t result_len = end < start ? 0u : end - start;
-    Handle<JSArray> result_array =
-        isolate->factory()->NewJSArray(HOLEY_ELEMENTS, result_len, result_len);
-    DisallowHeapAllocation no_gc;
-    FixedArray* elements = FixedArray::cast(result_array->elements());
-    FixedArray* parameters = FixedArray::cast(receiver->elements());
-    uint32_t insertion_index = 0;
-    for (uint32_t i = start; i < end; i++) {
-      uint32_t entry = GetEntryForIndexImpl(isolate, *receiver, parameters, i,
-                                            ALL_PROPERTIES);
-      if (entry != kMaxUInt32 && HasEntryImpl(isolate, parameters, entry)) {
-        elements->set(insertion_index, *GetImpl(isolate, parameters, entry));
-      } else {
-        elements->set_the_hole(isolate, insertion_index);
-      }
-      insertion_index++;
-    }
-    return result_array;
-  }
-
   static Handle<SeededNumberDictionary> NormalizeImpl(
       Handle<JSObject> object, Handle<FixedArrayBase> elements) {
     Handle<FixedArray> arguments =

src/flag-definitions.h

@@ -739,6 +739,8 @@ DEFINE_BOOL(expose_trigger_failure, false, "expose trigger-failure extension")
 DEFINE_INT(stack_trace_limit, 10, "number of stack frames to capture")
 DEFINE_BOOL(builtins_in_stack_traces, false,
             "show built-in functions in stack traces")
+DEFINE_BOOL(enable_experimental_builtins, false,
+            "enable new csa-based experimental builtins")
 
 // builtins.cc
 DEFINE_BOOL(allow_unsafe_function_constructor, false,

src/runtime/runtime-array.cc

@@ -378,6 +378,45 @@ RUNTIME_FUNCTION(Runtime_GetArrayKeys) {
   return *isolate->factory()->NewJSArrayWithElements(keys);
 }
 
+RUNTIME_FUNCTION(Runtime_TrySliceSimpleNonFastElements) {
+  HandleScope scope(isolate);
+  DCHECK_EQ(3, args.length());
+  CONVERT_ARG_HANDLE_CHECKED(JSReceiver, receiver, 0);
+  CONVERT_SMI_ARG_CHECKED(first, 1);
+  CONVERT_SMI_ARG_CHECKED(count, 2);
+  uint32_t length = first + count;
+
+  // Only handle elements kinds that have a ElementsAccessor Slice
+  // implementation.
+  if (receiver->IsJSArray()) {
+    // This "fastish" path must make sure the destination array is a JSArray.
+    if (!isolate->IsArraySpeciesLookupChainIntact() ||
+        !JSArray::cast(*receiver)->HasArrayPrototype(isolate)) {
+      return Smi::FromInt(0);
+    }
+  } else {
+    Map* map = receiver->map();
+    Context* context = *isolate->native_context();
+    if (map != context->sloppy_arguments_map() &&
+        map != context->strict_arguments_map() &&
+        map != context->fast_aliased_arguments_map() &&
+        map != context->slow_aliased_arguments_map()) {
+      return Smi::FromInt(0);
+    }
+  }
+
+  // This "fastish" path must also ensure that elements are simple (no
+  // geters/setters), no elements on prototype chain.
+  Handle<JSObject> object(Handle<JSObject>::cast(receiver));
+  if (!JSObject::PrototypeHasNoElements(isolate, *object) ||
+      object->HasComplexElements()) {
+    return Smi::FromInt(0);
+  }
+
+  ElementsAccessor* accessor = object->GetElementsAccessor();
+  return *accessor->Slice(object, first, length);
+}
+
 RUNTIME_FUNCTION(Runtime_NewArray) {
   HandleScope scope(isolate);
   DCHECK_LE(3, args.length());

src/runtime/runtime.h

@@ -36,22 +36,23 @@ namespace internal {
 // A variable number of arguments is specified by a -1, additional restrictions
 // are specified by inline comments
 
-#define FOR_EACH_INTRINSIC_ARRAY(F) \
-  F(TransitionElementsKind, 2, 1)   \
-  F(RemoveArrayHoles, 2, 1)         \
-  F(MoveArrayContents, 2, 1)        \
-  F(EstimateNumberOfElements, 1, 1) \
-  F(GetArrayKeys, 2, 1)             \
-  F(NewArray, -1 /* >= 3 */, 1)     \
-  F(FunctionBind, -1, 1)            \
-  F(NormalizeElements, 1, 1)        \
-  F(GrowArrayElements, 2, 1)        \
-  F(HasComplexElements, 1, 1)       \
-  F(IsArray, 1, 1)                  \
-  F(ArrayIsArray, 1, 1)             \
-  F(ArraySpeciesConstructor, 1, 1)  \
-  F(ArrayIncludes_Slow, 3, 1)       \
-  F(ArrayIndexOf, 3, 1)             \
+#define FOR_EACH_INTRINSIC_ARRAY(F)      \
+  F(TransitionElementsKind, 2, 1)        \
+  F(RemoveArrayHoles, 2, 1)              \
+  F(MoveArrayContents, 2, 1)             \
+  F(EstimateNumberOfElements, 1, 1)      \
+  F(GetArrayKeys, 2, 1)                  \
+  F(TrySliceSimpleNonFastElements, 3, 1) \
+  F(NewArray, -1 /* >= 3 */, 1)          \
+  F(FunctionBind, -1, 1)                 \
+  F(NormalizeElements, 1, 1)             \
+  F(GrowArrayElements, 2, 1)             \
+  F(HasComplexElements, 1, 1)            \
+  F(IsArray, 1, 1)                       \
+  F(ArrayIsArray, 1, 1)                  \
+  F(ArraySpeciesConstructor, 1, 1)       \
+  F(ArrayIncludes_Slow, 3, 1)            \
+  F(ArrayIndexOf, 3, 1)                  \
   F(SpreadIterablePrepare, 1, 1)
 
 #define FOR_EACH_INTRINSIC_ATOMICS(F)           \

test/cctest/compiler/test-run-stubs.cc

@@ -119,6 +119,119 @@ TEST(RunStringLengthStub) {
   CHECK_EQ(static_cast<int>(strlen(testString)), Smi::ToInt(*result));
 }
 
+TEST(RunArrayExtractStubSimple) {
+  HandleAndZoneScope scope;
+  Isolate* isolate = scope.main_isolate();
+  Zone* zone = scope.main_zone();
+
+  StubTester tester(isolate, zone, Builtins::kExtractFastJSArray);
+
+  // Actuall call through to the stub, verifying its result.
+  Handle<JSArray> source_array = isolate->factory()->NewJSArray(
+      PACKED_ELEMENTS, 5, 10, INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE);
+  static_cast<FixedArray*>(source_array->elements())->set(0, Smi::FromInt(5));
+  static_cast<FixedArray*>(source_array->elements())->set(1, Smi::FromInt(4));
+  static_cast<FixedArray*>(source_array->elements())->set(2, Smi::FromInt(3));
+  static_cast<FixedArray*>(source_array->elements())->set(3, Smi::FromInt(2));
+  static_cast<FixedArray*>(source_array->elements())->set(4, Smi::FromInt(1));
+  Handle<JSArray> result = Handle<JSArray>::cast(
+      tester.Call(source_array, Handle<Smi>(Smi::FromInt(0), isolate),
+                  Handle<Smi>(Smi::FromInt(5), isolate)));
+  CHECK_NE(*source_array, *result);
+  CHECK_EQ(result->GetElementsKind(), PACKED_ELEMENTS);
+  CHECK_EQ(static_cast<FixedArray*>(result->elements())->get(0),
+           Smi::FromInt(5));
+  CHECK_EQ(static_cast<FixedArray*>(result->elements())->get(1),
+           Smi::FromInt(4));
+  CHECK_EQ(static_cast<FixedArray*>(result->elements())->get(2),
+           Smi::FromInt(3));
+  CHECK_EQ(static_cast<FixedArray*>(result->elements())->get(3),
+           Smi::FromInt(2));
+  CHECK_EQ(static_cast<FixedArray*>(result->elements())->get(4),
+           Smi::FromInt(1));
+}
+
+TEST(RunArrayExtractDoubleStubSimple) {
+  HandleAndZoneScope scope;
+  Isolate* isolate = scope.main_isolate();
+  Zone* zone = scope.main_zone();
+
+  StubTester tester(isolate, zone, Builtins::kExtractFastJSArray);
+
+  // Actuall call through to the stub, verifying its result.
+  Handle<JSArray> source_array = isolate->factory()->NewJSArray(
+      PACKED_DOUBLE_ELEMENTS, 5, 10, INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE);
+  static_cast<FixedDoubleArray*>(source_array->elements())->set(0, 5);
+  static_cast<FixedDoubleArray*>(source_array->elements())->set(1, 4);
+  static_cast<FixedDoubleArray*>(source_array->elements())->set(2, 3);
+  static_cast<FixedDoubleArray*>(source_array->elements())->set(3, 2);
+  static_cast<FixedDoubleArray*>(source_array->elements())->set(4, 1);
+  Handle<JSArray> result = Handle<JSArray>::cast(
+      tester.Call(source_array, Handle<Smi>(Smi::FromInt(0), isolate),
+                  Handle<Smi>(Smi::FromInt(5), isolate)));
+  CHECK_NE(*source_array, *result);
+  CHECK_EQ(result->GetElementsKind(), PACKED_DOUBLE_ELEMENTS);
+  CHECK_EQ(static_cast<FixedDoubleArray*>(result->elements())->get_scalar(0),
+           5);
+  CHECK_EQ(static_cast<FixedDoubleArray*>(result->elements())->get_scalar(1),
+           4);
+  CHECK_EQ(static_cast<FixedDoubleArray*>(result->elements())->get_scalar(2),
+           3);
+  CHECK_EQ(static_cast<FixedDoubleArray*>(result->elements())->get_scalar(3),
+           2);
+  CHECK_EQ(static_cast<FixedDoubleArray*>(result->elements())->get_scalar(4),
+           1);
+}
+
+TEST(RunArrayExtractStubTooBigForNewSpace) {
+  HandleAndZoneScope scope;
+  Isolate* isolate = scope.main_isolate();
+  Zone* zone = scope.main_zone();
+
+  StubTester tester(isolate, zone, Builtins::kExtractFastJSArray);
+
+  // Actuall call through to the stub, verifying its result.
+  Handle<JSArray> source_array = isolate->factory()->NewJSArray(
+      PACKED_ELEMENTS, 500000, 500000, INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE);
+  for (int i = 0; i < 500000; ++i) {
+    static_cast<FixedArray*>(source_array->elements())->set(i, Smi::FromInt(i));
+  }
+  Handle<JSArray> result = Handle<JSArray>::cast(
+      tester.Call(source_array, Handle<Smi>(Smi::FromInt(0), isolate),
+                  Handle<Smi>(Smi::FromInt(500000), isolate)));
+  CHECK_NE(*source_array, *result);
+  CHECK_EQ(result->GetElementsKind(), PACKED_ELEMENTS);
+  for (int i = 0; i < 500000; ++i) {
+    CHECK_EQ(static_cast<FixedArray*>(source_array->elements())->get(i),
+             static_cast<FixedArray*>(result->elements())->get(i));
+  }
+}
+
+TEST(RunArrayExtractDoubleStubTooBigForNewSpace) {
+  HandleAndZoneScope scope;
+  Isolate* isolate = scope.main_isolate();
+  Zone* zone = scope.main_zone();
+
+  StubTester tester(isolate, zone, Builtins::kExtractFastJSArray);
+
+  // Actuall call through to the stub, verifying its result.
+  Handle<JSArray> source_array = isolate->factory()->NewJSArray(
+      PACKED_DOUBLE_ELEMENTS, 500000, 500000,
+      INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE, TENURED);
+  for (int i = 0; i < 500000; ++i) {
+    static_cast<FixedDoubleArray*>(source_array->elements())->set(i, i);
+  }
+  Handle<JSArray> result = Handle<JSArray>::cast(
+      tester.Call(source_array, Handle<Smi>(Smi::FromInt(0), isolate),
+                  Handle<Smi>(Smi::FromInt(500000), isolate)));
+  CHECK_NE(*source_array, *result);
+  CHECK_EQ(result->GetElementsKind(), PACKED_DOUBLE_ELEMENTS);
+  for (int i = 0; i < 500000; ++i) {
+    CHECK_EQ(
+        static_cast<FixedDoubleArray*>(source_array->elements())->get_scalar(i),
+        static_cast<FixedDoubleArray*>(result->elements())->get_scalar(i));
+  }
+}
 
 }  // namespace compiler
 }  // namespace internal

test/mjsunit/mjsunit.status

@@ -48,6 +48,9 @@
   # This test non-deterministically runs out of memory on Windows ia32.
   'regress/regress-crbug-160010': [SKIP],
 
+  # This test should be skipped until the new CSA-based slice is activated.
+  'splice-proxy': [SKIP],
+
   # Issue 3784: setters-on-elements is flaky
   'setters-on-elements': [PASS, FAIL],
 

test/mjsunit/splice-proxy.js

+// Copyright 2017 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.
+
+var array = [];
+var proxy = new Proxy(new Proxy(array, {}), {});
+var Ctor = function() {};
+var result;
+
+array.constructor = function() {};
+array.constructor[Symbol.species] = Ctor;
+
+Array.prototype.slice.call(proxy);