Adding ElementsAccessor::Slice

- Move fast paths from builtins.cc ArraySlice to ElementsAccessor
- Handle more argument types in the fast path

BUG=

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

Cr-Commit-Position: refs/heads/master@{#30471}

diff --git a/src/builtins.cc b/src/builtins.cc
index 9e5631d84a4d815d564d7d0dda40ecb5d087f55b..8fe53d95761c9eb24bc5fe79af56865a9c3af8bb 100644 (file)
--- a/src/builtins.cc
+++ b/src/builtins.cc
@@ -134,7 +134,7 @@ BUILTIN_LIST_C(DEF_ARG_TYPE)
 
 
 #ifdef DEBUG
-static inline bool CalledAsConstructor(Isolate* isolate) {
+inline bool CalledAsConstructor(Isolate* isolate) {
   // Calculate the result using a full stack frame iterator and check
   // that the state of the stack is as we assume it to be in the
   // code below.
@@ -165,16 +165,25 @@ static inline bool CalledAsConstructor(Isolate* isolate) {
 // ----------------------------------------------------------------------------
 
 
-bool ClampedToInteger(Object* object, int* out) {
+inline bool ClampedToInteger(Object* object, int* out) {
   // This is an extended version of ECMA-262 7.1.11 handling signed values
   // Try to convert object to a number and clamp values to [kMinInt, kMaxInt]
   if (object->IsSmi()) {
     *out = Smi::cast(object)->value();
     return true;
   } else if (object->IsHeapNumber()) {
-    *out = FastD2IChecked(HeapNumber::cast(object)->value());
+    double value = HeapNumber::cast(object)->value();
+    if (std::isnan(value)) {
+      *out = 0;
+    } else if (value > kMaxInt) {
+      *out = kMaxInt;
+    } else if (value < kMinInt) {
+      *out = kMinInt;
+    } else {
+      *out = static_cast<int>(value);
+    }
     return true;
-  } else if (object->IsUndefined()) {
+  } else if (object->IsUndefined() || object->IsNull()) {
     *out = 0;
     return true;
   } else if (object->IsBoolean()) {
@@ -185,15 +194,31 @@ bool ClampedToInteger(Object* object, int* out) {
 }
 
 
-static void MoveDoubleElements(FixedDoubleArray* dst, int dst_index,
-                               FixedDoubleArray* src, int src_index, int len) {
+void MoveDoubleElements(FixedDoubleArray* dst, int dst_index,
+                        FixedDoubleArray* src, int src_index, int len) {
   if (len == 0) return;
   MemMove(dst->data_start() + dst_index, src->data_start() + src_index,
           len * kDoubleSize);
 }
 
 
-static bool ArrayPrototypeHasNoElements(PrototypeIterator* iter) {
+inline bool GetSloppyArgumentsLength(Isolate* isolate, Handle<JSObject> object,
+                                     int* out) {
+  Map* arguments_map =
+      isolate->context()->native_context()->sloppy_arguments_map();
+  if (object->map() != arguments_map || !object->HasFastElements()) {
+    return false;
+  }
+  Object* len_obj = object->InObjectPropertyAt(Heap::kArgumentsLengthIndex);
+  if (!len_obj->IsSmi()) {
+    return false;
+  }
+  *out = Smi::cast(len_obj)->value();
+  return *out <= object->elements()->length();
+}
+
+
+bool PrototypeHasNoElements(PrototypeIterator* iter) {
   DisallowHeapAllocation no_gc;
   for (; !iter->IsAtEnd(); iter->Advance()) {
     if (iter->GetCurrent()->IsJSProxy()) return false;
@@ -206,8 +231,8 @@ static bool ArrayPrototypeHasNoElements(PrototypeIterator* iter) {
 }
 
 
-static inline bool IsJSArrayFastElementMovingAllowed(Isolate* isolate,
-                                                     JSArray* receiver) {
+inline bool IsJSArrayFastElementMovingAllowed(Isolate* isolate,
+                                              JSArray* receiver) {
   DisallowHeapAllocation no_gc;
   // If the array prototype chain is intact (and free of elements), and if the
   // receiver's prototype is the array prototype, then we are done.
@@ -220,16 +245,14 @@ static inline bool IsJSArrayFastElementMovingAllowed(Isolate* isolate,
 
   // Slow case.
   PrototypeIterator iter(isolate, receiver);
-  return ArrayPrototypeHasNoElements(&iter);
+  return PrototypeHasNoElements(&iter);
 }
 
 
 // Returns empty handle if not applicable.
 MUST_USE_RESULT
-static inline MaybeHandle<FixedArrayBase> EnsureJSArrayWithWritableFastElements(
-    Isolate* isolate,
-    Handle<Object> receiver,
-    Arguments* args,
+inline MaybeHandle<FixedArrayBase> EnsureJSArrayWithWritableFastElements(
+    Isolate* isolate, Handle<Object> receiver, Arguments* args,
     int first_added_arg) {
   if (!receiver->IsJSArray()) return MaybeHandle<FixedArrayBase>();
   Handle<JSArray> array = Handle<JSArray>::cast(receiver);
@@ -495,140 +518,79 @@ BUILTIN(ArrayUnshift) {
 BUILTIN(ArraySlice) {
   HandleScope scope(isolate);
   Handle<Object> receiver = args.receiver();
+  Handle<JSObject> object;
+  Handle<FixedArrayBase> elms_obj;
   int len = -1;
   int relative_start = 0;
   int relative_end = 0;
-  {
-    DisallowHeapAllocation no_gc;
-    if (receiver->IsJSArray()) {
-      JSArray* array = JSArray::cast(*receiver);
-      if (!IsJSArrayFastElementMovingAllowed(isolate, array)) {
-        AllowHeapAllocation allow_allocation;
-        return CallJsIntrinsic(isolate, isolate->array_slice(), args);
-      }
-
-      if (!array->HasFastElements()) {
-        AllowHeapAllocation allow_allocation;
-        return CallJsIntrinsic(isolate, isolate->array_slice(), args);
-      }
+  bool is_sloppy_arguments = false;
 
-      len = Smi::cast(array->length())->value();
-    } else {
-      // Array.slice(arguments, ...) is quite a common idiom (notably more
-      // than 50% of invocations in Web apps).  Treat it in C++ as well.
-      Map* arguments_map =
-          isolate->context()->native_context()->sloppy_arguments_map();
-
-      bool is_arguments_object_with_fast_elements =
-          receiver->IsJSObject() &&
-          JSObject::cast(*receiver)->map() == arguments_map;
-      if (!is_arguments_object_with_fast_elements) {
-        AllowHeapAllocation allow_allocation;
-        return CallJsIntrinsic(isolate, isolate->array_slice(), args);
-      }
-      JSObject* object = JSObject::cast(*receiver);
-
-      if (!object->HasFastElements()) {
-        AllowHeapAllocation allow_allocation;
-        return CallJsIntrinsic(isolate, isolate->array_slice(), args);
-      }
-
-      Object* len_obj = object->InObjectPropertyAt(Heap::kArgumentsLengthIndex);
-      if (!len_obj->IsSmi()) {
-        AllowHeapAllocation allow_allocation;
-        return CallJsIntrinsic(isolate, isolate->array_slice(), args);
-      }
-      len = Smi::cast(len_obj)->value();
-      if (len > object->elements()->length()) {
-        AllowHeapAllocation allow_allocation;
-        return CallJsIntrinsic(isolate, isolate->array_slice(), args);
-      }
+  if (receiver->IsJSArray()) {
+    DisallowHeapAllocation no_gc;
+    JSArray* array = JSArray::cast(*receiver);
+    if (!array->HasFastElements() ||
+        !IsJSArrayFastElementMovingAllowed(isolate, array)) {
+      AllowHeapAllocation allow_allocation;
+      return CallJsIntrinsic(isolate, isolate->array_slice(), args);
     }
-
-    DCHECK(len >= 0);
-    int n_arguments = args.length() - 1;
-
-    // Note carefully choosen defaults---if argument is missing,
-    // it's undefined which gets converted to 0 for relative_start
-    // and to len for relative_end.
-    relative_start = 0;
-    relative_end = len;
-    if (n_arguments > 0) {
-      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 CallJsIntrinsic(isolate, isolate->array_slice(), args);
-        }
-        relative_start = std::isnan(start) ? 0 : static_cast<int>(start);
-      } else if (!arg1->IsUndefined()) {
+    len = Smi::cast(array->length())->value();
+    object = Handle<JSObject>::cast(receiver);
+    elms_obj = handle(array->elements(), isolate);
+  } else if (receiver->IsJSObject() &&
+             GetSloppyArgumentsLength(isolate, Handle<JSObject>::cast(receiver),
+                                      &len)) {
+    // Array.prototype.slice(arguments, ...) is quite a common idiom
+    // (notably more than 50% of invocations in Web apps).
+    // Treat it in C++ as well.
+    is_sloppy_arguments = true;
+    object = Handle<JSObject>::cast(receiver);
+    elms_obj = handle(object->elements(), isolate);
+  } else {
+    AllowHeapAllocation allow_allocation;
+    return CallJsIntrinsic(isolate, isolate->array_slice(), args);
+  }
+  DCHECK(len >= 0);
+  int argument_count = args.length() - 1;
+  // Note carefully chosen defaults---if argument is missing,
+  // it's undefined which gets converted to 0 for relative_start
+  // and to len for relative_end.
+  relative_start = 0;
+  relative_end = len;
+  if (argument_count > 0) {
+    DisallowHeapAllocation no_gc;
+    if (!ClampedToInteger(args[1], &relative_start)) {
+      AllowHeapAllocation allow_allocation;
+      return CallJsIntrinsic(isolate, isolate->array_slice(), args);
+    }
+    if (argument_count > 1) {
+      Object* end_arg = args[2];
+      // slice handles the end_arg specially
+      if (end_arg->IsUndefined()) {
+        relative_end = len;
+      } else if (!ClampedToInteger(end_arg, &relative_end)) {
         AllowHeapAllocation allow_allocation;
         return CallJsIntrinsic(isolate, isolate->array_slice(), args);
       }
-      if (n_arguments > 1) {
-        Object* arg2 = args[2];
-        if (arg2->IsSmi()) {
-          relative_end = Smi::cast(arg2)->value();
-        } else if (arg2->IsHeapNumber()) {
-          double end = HeapNumber::cast(arg2)->value();
-          if (end < kMinInt || end > kMaxInt) {
-            AllowHeapAllocation allow_allocation;
-            return CallJsIntrinsic(isolate, isolate->array_slice(), args);
-          }
-          relative_end = std::isnan(end) ? 0 : static_cast<int>(end);
-        } else if (!arg2->IsUndefined()) {
-          AllowHeapAllocation allow_allocation;
-          return CallJsIntrinsic(isolate, isolate->array_slice(), args);
-        }
-      }
     }
   }
 
   // ECMAScript 232, 3rd Edition, Section 15.4.4.10, step 6.
-  int k = (relative_start < 0) ? Max(len + relative_start, 0)
-                               : Min(relative_start, len);
+  uint32_t actual_start = (relative_start < 0) ? Max(len + relative_start, 0)
+                                               : Min(relative_start, len);
 
   // ECMAScript 232, 3rd Edition, Section 15.4.4.10, step 8.
-  int final = (relative_end < 0) ? Max(len + relative_end, 0)
-                                 : Min(relative_end, len);
-
-  // Calculate the length of result array.
-  int result_len = Max(final - k, 0);
+  uint32_t actual_end =
+      (relative_end < 0) ? Max(len + relative_end, 0) : Min(relative_end, len);
 
-  Handle<JSObject> object = Handle<JSObject>::cast(receiver);
-  Handle<FixedArrayBase> elms(object->elements(), isolate);
-
-  ElementsKind kind = object->GetElementsKind();
-  if (IsHoleyElementsKind(kind)) {
-    DisallowHeapAllocation no_gc;
-    bool packed = true;
-    ElementsAccessor* accessor = ElementsAccessor::ForKind(kind);
-    for (int i = k; i < final; i++) {
-      if (!accessor->HasElement(object, i, elms)) {
-        packed = false;
-        break;
-      }
-    }
-    if (packed) {
-      kind = GetPackedElementsKind(kind);
-    } else if (!receiver->IsJSArray()) {
-      AllowHeapAllocation allow_allocation;
-      return CallJsIntrinsic(isolate, isolate->array_slice(), args);
-    }
+  ElementsAccessor* accessor = object->GetElementsAccessor();
+  if (is_sloppy_arguments &&
+      !accessor->IsPacked(object, elms_obj, actual_start, actual_end)) {
+    // Don't deal with arguments with holes in C++
+    AllowHeapAllocation allow_allocation;
+    return CallJsIntrinsic(isolate, isolate->array_slice(), args);
   }
-
   Handle<JSArray> result_array =
-      isolate->factory()->NewJSArray(kind, result_len, result_len);
-
-  DisallowHeapAllocation no_gc;
-  if (result_len == 0) return *result_array;
-
-  ElementsAccessor* accessor = object->GetElementsAccessor();
-  accessor->CopyElements(
-      elms, k, kind, handle(result_array->elements(), isolate), 0, result_len);
+      accessor->Slice(object, elms_obj, actual_start, actual_end);
   return *result_array;
 }
 
@@ -687,9 +649,9 @@ BUILTIN(ArraySplice) {
     return CallJsIntrinsic(isolate, isolate->array_splice(), args);
   }
   ElementsAccessor* accessor = array->GetElementsAccessor();
-  Handle<JSArray> result = accessor->Splice(
+  Handle<JSArray> result_array = accessor->Splice(
       array, elms_obj, actual_start, actual_delete_count, args, add_count);
-  return *result;
+  return *result_array;
 }
 
 
@@ -706,7 +668,7 @@ BUILTIN(ArrayConcat) {
     Object* array_proto = native_context->array_function()->prototype();
     PrototypeIterator iter(isolate, array_proto,
                            PrototypeIterator::START_AT_RECEIVER);
-    if (!ArrayPrototypeHasNoElements(&iter)) {
+    if (!PrototypeHasNoElements(&iter)) {
       AllowHeapAllocation allow_allocation;
       return CallJsIntrinsic(isolate, isolate->array_concat(), args);
     }

diff --git a/src/elements.cc b/src/elements.cc
index 921c371e34b27d9dd0397f14eb614cd1d9c109f8..d17efafcebb2c6c88e585381ffda311db20d63bc 100644 (file)
--- a/src/elements.cc
+++ b/src/elements.cc
@@ -515,8 +515,33 @@ class ElementsAccessorBase : public ElementsAccessor {
     ElementsAccessorSubclass::ValidateImpl(holder);
   }
 
+  virtual bool IsPacked(Handle<JSObject> holder,
+                        Handle<FixedArrayBase> backing_store, uint32_t start,
+                        uint32_t end) final {
+    return ElementsAccessorSubclass::IsPackedImpl(holder, backing_store, start,
+                                                  end);
+  }
+
+  static bool IsPackedImpl(Handle<JSObject> holder,
+                           Handle<FixedArrayBase> backing_store, uint32_t start,
+                           uint32_t end) {
+    if (IsFastPackedElementsKind(kind())) return true;
+    for (uint32_t i = start; i < end; i++) {
+      if (!ElementsAccessorSubclass::HasElementImpl(holder, i, backing_store)) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   virtual bool HasElement(Handle<JSObject> holder, uint32_t index,
                           Handle<FixedArrayBase> backing_store) final {
+    return ElementsAccessorSubclass::HasElementImpl(holder, index,
+                                                    backing_store);
+  }
+
+  static bool HasElementImpl(Handle<JSObject> holder, uint32_t index,
+                             Handle<FixedArrayBase> backing_store) {
     return ElementsAccessorSubclass::GetEntryForIndexImpl(
                *holder, *backing_store, index) != kMaxUInt32;
   }
@@ -572,7 +597,7 @@ class ElementsAccessorBase : public ElementsAccessor {
 
   virtual uint32_t Push(Handle<JSArray> receiver,
                         Handle<FixedArrayBase> backing_store, Object** objects,
-                        uint32_t push_size, int direction) {
+                        uint32_t push_size, int direction) final {
     return ElementsAccessorSubclass::PushImpl(receiver, backing_store, objects,
                                               push_size, direction);
   }
@@ -584,10 +609,24 @@ class ElementsAccessorBase : public ElementsAccessor {
     return 0;
   }
 
+  virtual Handle<JSArray> Slice(Handle<JSObject> receiver,
+                                Handle<FixedArrayBase> backing_store,
+                                uint32_t start, uint32_t end) final {
+    return ElementsAccessorSubclass::SliceImpl(receiver, backing_store, start,
+                                               end);
+  }
+
+  static Handle<JSArray> SliceImpl(Handle<JSObject> receiver,
+                                   Handle<FixedArrayBase> backing_store,
+                                   uint32_t start, uint32_t end) {
+    UNREACHABLE();
+    return Handle<JSArray>();
+  }
+
   virtual Handle<JSArray> Splice(Handle<JSArray> receiver,
                                  Handle<FixedArrayBase> backing_store,
                                  uint32_t start, uint32_t delete_count,
-                                 Arguments args, uint32_t add_count) {
+                                 Arguments args, uint32_t add_count) final {
     return ElementsAccessorSubclass::SpliceImpl(receiver, backing_store, start,
                                                 delete_count, args, add_count);
   }
@@ -1232,14 +1271,31 @@ class FastElementsAccessor
     UNREACHABLE();
   }
 
+  static Handle<JSArray> SliceImpl(Handle<JSObject> receiver,
+                                   Handle<FixedArrayBase> backing_store,
+                                   uint32_t start, uint32_t end) {
+    Isolate* isolate = receiver->GetIsolate();
+    if (end <= start) {
+      return isolate->factory()->NewJSArray(KindTraits::Kind, 0, 0);
+    }
+    int result_len = end - start;
+    Handle<JSArray> result_array = isolate->factory()->NewJSArray(
+        KindTraits::Kind, result_len, result_len);
+    DisallowHeapAllocation no_gc;
+    FastElementsAccessorSubclass::CopyElementsImpl(
+        *backing_store, start, result_array->elements(), KindTraits::Kind, 0,
+        kPackedSizeNotKnown, result_len);
+    return result_array;
+  }
+
   static Handle<JSArray> SpliceImpl(Handle<JSArray> receiver,
                                     Handle<FixedArrayBase> backing_store,
                                     uint32_t start, uint32_t delete_count,
                                     Arguments args, uint32_t add_count) {
     Isolate* isolate = receiver->GetIsolate();
     Heap* heap = isolate->heap();
-    const uint32_t len = Smi::cast(receiver->length())->value();
-    const uint32_t new_length = len - delete_count + add_count;
+    uint32_t len = Smi::cast(receiver->length())->value();
+    uint32_t new_length = len - delete_count + add_count;
 
     if (new_length == 0) {
       receiver->set_elements(heap->empty_fixed_array());

diff --git a/src/elements.h b/src/elements.h
index 16be8ecde3564941735e5e093ad920ec4d9e856e..f5a9b3e23a6b306021254f83ef94c983dc3a0e6c 100644 (file)
--- a/src/elements.h
+++ b/src/elements.h
@@ -38,6 +38,11 @@ class ElementsAccessor {
     return HasElement(holder, index, handle(holder->elements()));
   }
 
+  // Returns true if the backing store is compact in the given range
+  virtual bool IsPacked(Handle<JSObject> holder,
+                        Handle<FixedArrayBase> backing_store, uint32_t start,
+                        uint32_t end) = 0;
+
   virtual Handle<Object> Get(Handle<FixedArrayBase> backing_store,
                              uint32_t entry) = 0;
 
@@ -131,6 +136,10 @@ class ElementsAccessor {
                         Handle<FixedArrayBase> backing_store, Object** objects,
                         uint32_t start, int direction) = 0;
 
+  virtual Handle<JSArray> Slice(Handle<JSObject> receiver,
+                                Handle<FixedArrayBase> backing_store,
+                                uint32_t start, uint32_t end) = 0;
+
   virtual Handle<JSArray> Splice(Handle<JSArray> receiver,
                                  Handle<FixedArrayBase> backing_store,
                                  uint32_t start, uint32_t delete_count,

diff --git a/test/mjsunit/regress/regress-165637.js b/test/mjsunit/regress/regress-165637.js
index 84c9041216cb6045c036054b67e470c7615f5639..6e287263561b751ece32bb420e58b437ba5a8781 100644 (file)
--- a/test/mjsunit/regress/regress-165637.js
+++ b/test/mjsunit/regress/regress-165637.js
@@ -51,7 +51,7 @@ assertTrue(do_slices() < (3 * 1000));
 
 // Make sure that packed and unpacked array slices are still properly handled
 var holey_array = [1, 2, 3, 4, 5,,,,,,];
-assertFalse(%HasFastHoleyElements(holey_array.slice(6, 1)));
-assertEquals(undefined, holey_array.slice(6, 7)[0])
-assertFalse(%HasFastHoleyElements(holey_array.slice(2, 1)));
-assertEquals(3, holey_array.slice(2, 3)[0])
+assertEquals([undefined], holey_array.slice(6, 7));
+assertEquals(undefined, holey_array.slice(6, 7)[0]);
+assertEquals([], holey_array.slice(2, 1));
+assertEquals(3, holey_array.slice(2, 3)[0]);