Bug 1377279 - Fine-tune array built-ins. r=djvj
authorAndré Bargull <andre.bargull@gmail.com>
Thu, 29 Jun 2017 14:59:14 -0700
changeset 418711 52da209e46fbcb3e7913ef28c0d4762ed13f8a62
parent 418710 8000ce4433095f60619118fd2b19259e640b9062
child 418712 bb2904560e11fcaded572b00e0b1f434844a43c5
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdjvj
bugs1377279
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1377279 - Fine-tune array built-ins. r=djvj
js/src/builtin/Sorting.js
js/src/jsarray.cpp
js/src/jsarray.h
--- a/js/src/builtin/Sorting.js
+++ b/js/src/builtin/Sorting.js
@@ -267,22 +267,22 @@ function MergeSort(array, len, comparefn
     // typed array specific sorting method it makes sense to divert to it
     // when possible.
     if (IsPossiblyWrappedTypedArray(array)) {
         return callFunction(TypedArraySort, array, comparefn);
     }
 
     // To save effort we will do all of our work on a dense list,
     // then create holes at the end.
-    var denseList = new List();
+    var denseList = [];
     var denseLen = 0;
 
     for (var i = 0; i < len; i++) {
         if (i in array)
-            denseList[denseLen++] = array[i];
+            _DefineDataProperty(denseList, denseLen++, array[i]);
     }
 
     if (denseLen < 1)
         return array;
 
     // Insertion sort for small arrays, where "small" is defined by performance
     // testing.
     if (denseLen < 24) {
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -398,21 +398,31 @@ js::GetElementsWithAdder(JSContext* cx, 
         }
         if (!adder->append(cx, val))
             return false;
     }
 
     return true;
 }
 
+static bool
+ObjectMayHaveExtraIndexedProperties(JSObject* obj);
+
+static inline bool
+IsPackedArrayOrNoExtraIndexedProperties(JSObject* obj, uint64_t length)
+{
+    return (IsPackedArray(obj) && obj->as<ArrayObject>().length() == length) ||
+           !ObjectMayHaveExtraIndexedProperties(obj);
+}
+
 template <JSValueType Type>
 DenseElementResult
 GetBoxedOrUnboxedDenseElements(JSObject* aobj, uint32_t length, Value* vp)
 {
-    MOZ_ASSERT(!ObjectMayHaveExtraIndexedProperties(aobj));
+    MOZ_ASSERT(IsPackedArrayOrNoExtraIndexedProperties(aobj, length));
 
     if (length > GetBoxedOrUnboxedInitializedLength<Type>(aobj))
         return DenseElementResult::Incomplete;
 
     for (size_t i = 0; i < length; i++) {
         vp[i] = GetBoxedOrUnboxedDenseElement<Type>(aobj, i);
 
         // No other indexed properties so hole => undefined.
@@ -424,17 +434,17 @@ GetBoxedOrUnboxedDenseElements(JSObject*
 }
 
 DefineBoxedOrUnboxedFunctor3(GetBoxedOrUnboxedDenseElements,
                              JSObject*, uint32_t, Value*);
 
 bool
 js::GetElements(JSContext* cx, HandleObject aobj, uint32_t length, Value* vp)
 {
-    if (!ObjectMayHaveExtraIndexedProperties(aobj)) {
+    if (IsPackedArrayOrNoExtraIndexedProperties(aobj, length)) {
         GetBoxedOrUnboxedDenseElementsFunctor functor(aobj, length, vp);
         DenseElementResult result = CallBoxedOrUnboxedSpecialization(functor, aobj);
         if (result != DenseElementResult::Incomplete)
             return result == DenseElementResult::Success;
     }
 
     if (aobj->is<ArgumentsObject>()) {
         ArgumentsObject& argsobj = aobj->as<ArgumentsObject>();
@@ -553,28 +563,46 @@ DeletePropertyOrThrow(JSContext* cx, Han
         if (!ToId(cx, index, &id))
             return false;
         return success.reportError(cx, obj, id);
     }
     return true;
 }
 
 static bool
+SetArrayLengthProperty(JSContext* cx, HandleArrayObject obj, HandleValue value)
+{
+    RootedId id(cx, NameToId(cx->names().length));
+    ObjectOpResult result;
+    if (obj->lengthIsWritable()) {
+        if (!ArraySetLength(cx, obj, id, JSPROP_PERMANENT, value, result))
+            return false;
+    } else {
+        MOZ_ALWAYS_TRUE(result.fail(JSMSG_READ_ONLY));
+    }
+    return result.checkStrict(cx, obj, id);
+}
+
+static bool
 SetLengthProperty(JSContext* cx, HandleObject obj, uint64_t length)
 {
     MOZ_ASSERT(length < uint64_t(DOUBLE_INTEGRAL_PRECISION_LIMIT));
 
     RootedValue v(cx, NumberValue(length));
+    if (obj->is<ArrayObject>())
+        return SetArrayLengthProperty(cx, obj.as<ArrayObject>(), v);
     return SetProperty(cx, obj, cx->names().length, v);
 }
 
 bool
 js::SetLengthProperty(JSContext* cx, HandleObject obj, uint32_t length)
 {
     RootedValue v(cx, NumberValue(length));
+    if (obj->is<ArrayObject>())
+        return SetArrayLengthProperty(cx, obj.as<ArrayObject>(), v);
     return SetProperty(cx, obj, cx->names().length, v);
 }
 
 static bool
 array_length_getter(JSContext* cx, HandleObject obj, HandleId id, MutableHandleValue vp)
 {
     vp.setNumber(obj->as<ArrayObject>().length());
     return true;
@@ -937,18 +965,18 @@ ObjectMayHaveExtraIndexedOwnProperties(J
                              obj->getClass(), INT_TO_JSID(0), obj);
 }
 
 /*
  * Whether obj may have indexed properties anywhere besides its dense
  * elements. This includes other indexed properties in its shape hierarchy, and
  * indexed properties or elements along its prototype chain.
  */
-bool
-js::ObjectMayHaveExtraIndexedProperties(JSObject* obj)
+static bool
+ObjectMayHaveExtraIndexedProperties(JSObject* obj)
 {
     MOZ_ASSERT_IF(obj->hasDynamicPrototype(), !obj->isNative());
 
     if (ObjectMayHaveExtraIndexedOwnProperties(obj))
         return true;
 
     do {
         MOZ_ASSERT(obj->hasStaticPrototype(),
@@ -1249,17 +1277,17 @@ struct ArrayJoinDenseKernelFunctor {
 template <typename SeparatorOp>
 static bool
 ArrayJoinKernel(JSContext* cx, SeparatorOp sepOp, HandleObject obj, uint64_t length,
                StringBuffer& sb)
 {
     // Step 6.
     uint32_t numProcessed = 0;
 
-    if (!ObjectMayHaveExtraIndexedProperties(obj)) {
+    if (IsPackedArrayOrNoExtraIndexedProperties(obj, length)) {
         ArrayJoinDenseKernelFunctor<SeparatorOp> functor(cx, sepOp, obj, length, sb, &numProcessed);
         DenseElementResult result = CallBoxedOrUnboxedSpecialization(functor, obj);
         if (result == DenseElementResult::Failure)
             return false;
     }
 
     // Step 7.
     if (numProcessed != length) {
@@ -1490,31 +1518,37 @@ ArrayReverseDenseKernel(JSContext* cx, H
     /* An empty array or an array with no elements is already reversed. */
     if (length == 0 || GetBoxedOrUnboxedInitializedLength<Type>(obj) == 0)
         return DenseElementResult::Success;
 
     if (Type == JSVAL_TYPE_MAGIC) {
         if (obj->as<NativeObject>().denseElementsAreFrozen())
             return DenseElementResult::Incomplete;
 
-        /*
-         * It's actually surprisingly complicated to reverse an array due to the
-         * orthogonality of array length and array capacity while handling
-         * leading and trailing holes correctly.  Reversing seems less likely to
-         * be a common operation than other array mass-mutation methods, so for
-         * now just take a probably-small memory hit (in the absence of too many
-         * holes in the array at its start) and ensure that the capacity is
-         * sufficient to hold all the elements in the array if it were full.
-         */
-        DenseElementResult result = obj->as<NativeObject>().ensureDenseElements(cx, length, 0);
-        if (result != DenseElementResult::Success)
-            return result;
-
-        /* Fill out the array's initialized length to its proper length. */
-        obj->as<NativeObject>().ensureDenseInitializedLength(cx, length, 0);
+        if (!IsPackedArray(obj)) {
+            /*
+             * It's actually surprisingly complicated to reverse an array due
+             * to the orthogonality of array length and array capacity while
+             * handling leading and trailing holes correctly.  Reversing seems
+             * less likely to be a common operation than other array
+             * mass-mutation methods, so for now just take a probably-small
+             * memory hit (in the absence of too many holes in the array at
+             * its start) and ensure that the capacity is sufficient to hold
+             * all the elements in the array if it were full.
+             */
+            DenseElementResult result = obj->as<NativeObject>().ensureDenseElements(cx, length, 0);
+            if (result != DenseElementResult::Success)
+                return result;
+
+            /* Fill out the array's initialized length to its proper length. */
+            obj->as<NativeObject>().ensureDenseInitializedLength(cx, length, 0);
+        } else {
+            if (!obj->as<NativeObject>().maybeCopyElementsForWrite(cx))
+                return DenseElementResult::Failure;
+        }
     } else {
         // Unboxed arrays can only be reversed here if their initialized length
         // matches their actual length. Otherwise the reversal will place holes
         // at the beginning of the array, which we don't support.
         if (length != obj->as<UnboxedArrayObject>().initializedLength())
             return DenseElementResult::Incomplete;
     }
 
@@ -1557,17 +1591,17 @@ js::array_reverse(JSContext* cx, unsigne
     if (!obj)
         return false;
 
     // Step 2.
     uint64_t len;
     if (!GetLengthProperty(cx, obj, &len))
         return false;
 
-    if (!ObjectMayHaveExtraIndexedProperties(obj) && len <= UINT32_MAX) {
+    if (IsPackedArrayOrNoExtraIndexedProperties(obj, len) && len <= UINT32_MAX) {
         ArrayReverseDenseKernelFunctor functor(cx, obj, uint32_t(len));
         DenseElementResult result = CallBoxedOrUnboxedSpecialization(functor, obj);
         if (result != DenseElementResult::Incomplete) {
             /*
              * Per ECMA-262, don't update the length of the array, even if the new
              * array has trailing holes (and thus the original array began with
              * holes).
              */
@@ -2118,34 +2152,56 @@ js::array_sort(JSContext* cx, unsigned a
          * value undefined and that is always greater than any other property.
          * Thus to sort holes and undefs we simply count them, sort the rest
          * of elements, append undefs after them and then make holes after
          * undefs.
          */
         undefs = 0;
         bool allStrings = true;
         bool allInts = true;
-        bool extraIndexed = ObjectMayHaveExtraIndexedProperties(obj);
+        bool extraIndexed;
         RootedValue v(cx);
-        for (uint32_t i = 0; i < len; i++) {
-            if (!CheckForInterrupt(cx))
-                return false;
-
-            bool hole;
-            if (!HasAndGetElement(cx, obj, i, &hole, &v))
-                return false;
-            if (hole)
-                continue;
-            if (v.isUndefined()) {
-                ++undefs;
-                continue;
+        if (IsPackedArray(obj)) {
+            HandleArrayObject array = obj.as<ArrayObject>();
+            extraIndexed = false;
+
+            for (uint32_t i = 0; i < len; i++) {
+                if (!CheckForInterrupt(cx))
+                    return false;
+
+                v.set(array->getDenseElement(i));
+                MOZ_ASSERT(!v.isMagic(JS_ELEMENTS_HOLE));
+                if (v.isUndefined()) {
+                    ++undefs;
+                    continue;
+                }
+                vec.infallibleAppend(v);
+                allStrings = allStrings && v.isString();
+                allInts = allInts && v.isInt32();
             }
-            vec.infallibleAppend(v);
-            allStrings = allStrings && v.isString();
-            allInts = allInts && v.isInt32();
+        } else {
+            extraIndexed = ObjectMayHaveExtraIndexedProperties(obj);
+
+            for (uint32_t i = 0; i < len; i++) {
+                if (!CheckForInterrupt(cx))
+                    return false;
+
+                bool hole;
+                if (!HasAndGetElement(cx, obj, i, &hole, &v))
+                    return false;
+                if (hole)
+                    continue;
+                if (v.isUndefined()) {
+                    ++undefs;
+                    continue;
+                }
+                vec.infallibleAppend(v);
+                allStrings = allStrings && v.isString();
+                allInts = allInts && v.isInt32();
+            }
         }
 
         /*
          * If the array only contains holes, we're done.  But if it contains
          * undefs, those must be sorted to the front of the array.
          */
         n = vec.length();
         if (n == 0 && undefs == 0) {
@@ -2354,17 +2410,17 @@ js::ArrayShiftMoveElements(JSObject* obj
     ShiftMoveBoxedOrUnboxedDenseElementsFunctor functor(obj);
     JS_ALWAYS_TRUE(CallBoxedOrUnboxedSpecialization(functor, obj) == DenseElementResult::Success);
 }
 
 template <JSValueType Type>
 DenseElementResult
 ArrayShiftDenseKernel(JSContext* cx, HandleObject obj, MutableHandleValue rval)
 {
-    if (ObjectMayHaveExtraIndexedProperties(obj))
+    if (!IsPackedArray(obj) && ObjectMayHaveExtraIndexedProperties(obj))
         return DenseElementResult::Incomplete;
 
     if (MaybeInIteration(obj, cx))
         return DenseElementResult::Incomplete;
 
     size_t initlen = GetBoxedOrUnboxedInitializedLength<Type>(obj);
     if (initlen == 0)
         return DenseElementResult::Incomplete;
--- a/js/src/jsarray.h
+++ b/js/src/jsarray.h
@@ -141,19 +141,16 @@ extern bool
 CanonicalizeArrayLengthValue(JSContext* cx, HandleValue v, uint32_t* canonicalized);
 
 extern bool
 GetLengthProperty(JSContext* cx, HandleObject obj, uint32_t* lengthp);
 
 extern bool
 SetLengthProperty(JSContext* cx, HandleObject obj, uint32_t length);
 
-extern bool
-ObjectMayHaveExtraIndexedProperties(JSObject* obj);
-
 /*
  * Copy 'length' elements from aobj to vp.
  *
  * This function assumes 'length' is effectively the result of calling
  * GetLengthProperty on aobj. vp must point to rooted memory.
  */
 extern bool
 GetElements(JSContext* cx, HandleObject aobj, uint32_t length, js::Value* vp);