closes #509: M1364346 parts 2+3 (unshift)

js/src/jit-test/tests/basic/shifted-elements4-unshift.js

@@ -0,0 +1,11 @@
+function f() {
+    var arr = [];
+    for (var i = 0; i < 2; i++) {
+	for (var j = 0; j < 90000; j++)
+	    arr.unshift(j);
+	for (var j = 0; j < 90000; j++)
+	    assertEq(arr.pop(), j);
+	assertEq(arr.length, 0);
+    }
+}
+f();

js/src/jit-test/tests/basic/shifted-elements7.js

@@ -0,0 +1,47 @@
+function test1() {
+    var a = [];
+    for (var i = 0; i < 100; i++)
+        a.unshift("foo" + i);
+    for (var i = 99; i >= 0; i--) {
+        assertEq(a.shift(), "foo" + i);
+        a.unshift("foo" + (i - 1));
+    }
+    assertEq(a.length, 100);
+}
+test1();
+
+function sum(arr) {
+    var res = 0;
+    for (var i = 0; i < arr.length; i++)
+        res += arr[i];
+    return res;
+}
+function test2() {
+    var a = [];
+    for (var i = 0; i < 200; i++)
+        a.push(i);
+    for (var i = 0; i < 100; i++)
+        a.shift();
+    for (var i = 0; i < 200; i++)
+        a.unshift(i);
+    assertEq(a.length, 300);
+    assertEq(sum(a), 34850);
+}
+test2();
+
+function test3() {
+    var a = [];
+    for (var i = 0; i < 200; i++)
+        a.push(i);
+    var toAdd = [];
+    var step = 1;
+    for (var i = 0; i < 2500; i += step) {
+        for (var j = 0; j < step; j++)
+            toAdd.unshift(i + j);
+        a.unshift(...toAdd);
+        step = Math.max((i / 16)|0, 1);
+    }
+    assertEq(a.length, 41463);
+    assertEq(sum(a), 26657756);
+}
+test3();

js/src/jsarray.cpp

@@ -769,7 +769,7 @@ js::ArraySetLength(JSContext* cx, Handle<ArrayObject*> arr, HandleId id,
 
     if (attrs & JSPROP_READONLY) {
         if (header->numShiftedElements() > 0) {
-            arr->unshiftElements();
+            arr->moveShiftedElements();
             header = arr->getElementsHeader();
         }
 
@@ -2255,7 +2255,7 @@ js::array_unshift(JSContext* cx, unsigned argc, Value* vp)
     if (args.length() > 0) {
         /* Slide up the array to make room for all args at the bottom. */
         if (length > 0) {
-            // Only include a fast path for boxed arrays. Unboxed arrays can'nt
+            // Only include a fast path for boxed arrays. Unboxed arrays can't
             // be optimized here because unshifting temporarily places holes at
             // the start of the array.
             bool optimized = false;
@@ -2267,14 +2267,16 @@ js::array_unshift(JSContext* cx, unsigned argc, Value* vp)
                 ArrayObject* aobj = &obj->as<ArrayObject>();
                 if (!aobj->lengthIsWritable())
                     break;
-                DenseElementResult result = aobj->ensureDenseElements(cx, length, args.length());
-                if (result != DenseElementResult::Success) {
-                    if (result == DenseElementResult::Failure)
-                        return false;
-                    MOZ_ASSERT(result == DenseElementResult::Incomplete);
-                    break;
+                if (MOZ_UNLIKELY(length > UINT32_MAX) || !aobj->tryUnshiftDenseElements(args.length())) {
+                    DenseElementResult result = aobj->ensureDenseElements(cx, length, args.length());
+                    if (result != DenseElementResult::Success) {
+                        if (result == DenseElementResult::Failure)
+                            return false;
+                        MOZ_ASSERT(result == DenseElementResult::Incomplete);
+                        break;
+                    }
+                    aobj->moveDenseElements(args.length(), 0, length);
                 }
-                aobj->moveDenseElements(args.length(), 0, length);
                 for (uint32_t i = 0; i < args.length(); i++)
                     aobj->setDenseElement(i, MagicValue(JS_ELEMENTS_HOLE));
                 optimized = true;

js/src/jsobj.cpp

@@ -537,7 +537,7 @@ js::SetIntegrityLevel(JSContext* cx, HandleObject obj, IntegrityLevel level)
             if (!obj->as<ArrayObject>().maybeCopyElementsForWrite(cx))
                 return false;
             if (nobj->getElementsHeader()->numShiftedElements() > 0)
-                nobj->unshiftElements();
+                nobj->moveShiftedElements();
             obj->as<ArrayObject>().getElementsHeader()->setNonwritableArrayLength();
         }
     } else {

js/src/vm/NativeObject-inl.h

@@ -300,11 +300,19 @@ NativeObject::tryShiftDenseElements(uint32_t count)
         return false;
     }
 
+    shiftDenseElementsUnchecked(count);
+    return true;
+}
+
+inline void
+NativeObject::shiftDenseElementsUnchecked(uint32_t count)
+{
+    ObjectElements* header = getElementsHeader();
     MOZ_ASSERT(count > 0);
     MOZ_ASSERT(count < header->initializedLength);
 
     if (MOZ_UNLIKELY(header->numShiftedElements() + count > ObjectElements::MaxShiftedElements)) {
-        unshiftElements();
+        moveShiftedElements();
         header = getElementsHeader();
     }
 
@@ -314,7 +322,6 @@ NativeObject::tryShiftDenseElements(uint32_t count)
     elements_ += count;
     ObjectElements* newHeader = getElementsHeader();
     memmove(newHeader, header, sizeof(ObjectElements));
-    return true;
 }
 
 /* Make an object with pregenerated shape from a NEWOBJECT bytecode. */

js/src/vm/NativeObject.cpp

@@ -760,7 +760,7 @@ NativeObject::goodElementsAllocationAmount(ExclusiveContext* cx, uint32_t reqCap
 }
 
 void
-NativeObject::unshiftElements()
+NativeObject::moveShiftedElements()
 {
     ObjectElements* header = getElementsHeader();
     uint32_t numShifted = header->numShiftedElements();
@@ -776,7 +776,7 @@ NativeObject::unshiftElements()
     elements_ = newHeader->elements();
 
     // To move the elements, temporarily update initializedLength to include
-    // both shifted and unshifted elements.
+    // the shifted elements.
     newHeader->initializedLength += numShifted;
 
     // Move the elements. Initialize to |undefined| to ensure pre-barriers
@@ -792,14 +792,95 @@ NativeObject::unshiftElements()
 }
 
 void
-NativeObject::maybeUnshiftElements()
+NativeObject::maybeMoveShiftedElements()
 {
     ObjectElements* header = getElementsHeader();
     MOZ_ASSERT(header->numShiftedElements() > 0);
 
-    // Unshift if less than a third of the allocated space is in use.
+    // Move the elements if less than a third of the allocated space is in use.
     if (header->capacity < header->numAllocatedElements() / 3)
-        unshiftElements();
+        moveShiftedElements();
+}
+
+bool
+NativeObject::tryUnshiftDenseElements(uint32_t count)
+{
+    MOZ_ASSERT(count > 0);
+
+    ObjectElements* header = getElementsHeader();
+    uint32_t numShifted = header->numShiftedElements();
+
+    if (count > numShifted) {
+        // We need more elements than are easily available. Try to make space
+        // for more elements than we need (and shift the remaining ones) so
+        // that unshifting more elements later will be fast.
+
+        // Don't bother reserving elements if the number of elements is small.
+        // Note that there's no technical reason for using this particular
+        // limit.
+        if (header->initializedLength <= 10 ||
+            header->isCopyOnWrite() ||
+            // header->isFrozen() || // NYI
+            header->hasNonwritableArrayLength() ||
+            MOZ_UNLIKELY(count > ObjectElements::MaxShiftedElements))
+        {
+            return false;
+        }
+
+        MOZ_ASSERT(header->capacity >= header->initializedLength);
+        uint32_t unusedCapacity = header->capacity - header->initializedLength;
+
+        // Determine toShift, the number of extra elements we want to make
+        // available.
+        uint32_t toShift = count - numShifted;
+        MOZ_ASSERT(toShift <= ObjectElements::MaxShiftedElements,
+                   "count <= MaxShiftedElements so toShift <= MaxShiftedElements");
+
+        // Give up if we need to allocate more elements.
+        if (toShift > unusedCapacity)
+            return false;
+
+        // Move more elements than we need, so that other unshift calls will be
+        // fast. We just have to make sure we don't exceed unusedCapacity.
+        toShift = Min(toShift + unusedCapacity / 2, unusedCapacity);
+
+        // Ensure |numShifted + toShift| does not exceed MaxShiftedElements.
+        if (numShifted + toShift > ObjectElements::MaxShiftedElements)
+            toShift = ObjectElements::MaxShiftedElements - numShifted;
+
+        MOZ_ASSERT(count <= numShifted + toShift);
+        MOZ_ASSERT(numShifted + toShift <= ObjectElements::MaxShiftedElements);
+        MOZ_ASSERT(toShift <= unusedCapacity);
+
+        // Now move/unshift the elements.
+        uint32_t initLen = header->initializedLength;
+        setDenseInitializedLength(initLen + toShift);
+        for (uint32_t i = 0; i < toShift; i++)
+            initDenseElement(initLen + i, UndefinedValue());
+        moveDenseElements(toShift, 0, initLen);
+
+        // Shift the elements we just prepended.
+        shiftDenseElementsUnchecked(toShift);
+
+        // We can now fall-through to the fast path below.
+        header = getElementsHeader();
+        MOZ_ASSERT(header->numShiftedElements() == numShifted + toShift);
+
+        numShifted = header->numShiftedElements();
+        MOZ_ASSERT(count <= numShifted);
+    }
+
+    elements_ -= count;
+    ObjectElements* newHeader = getElementsHeader();
+    memmove(newHeader, header, sizeof(ObjectElements));
+
+    newHeader->unshiftShiftedElements(count);
+
+    // Initialize to |undefined| to ensure pre-barriers don't see garbage.
+    for (uint32_t i = 0; i < count; i++)
+        initDenseElement(i, UndefinedValue());
+
+    return true;
 }
 
 bool
@@ -810,22 +891,31 @@ NativeObject::growElements(ExclusiveContext* cx, uint32_t reqCapacity)
     if (denseElementsAreCopyOnWrite())
         MOZ_CRASH();
 
-    // If there are shifted elements, consider unshifting them first. If we
-    // don't unshift here, the code below will include the shifted elements in
-    // the resize.
+    // If there are shifted elements, consider moving them first. If we don't
+    // move them here, the code below will include the shifted elements in the
+    // resize.
     uint32_t numShifted = getElementsHeader()->numShiftedElements();
     if (numShifted > 0) {
-        maybeUnshiftElements();
+        // If the number of elements is small, it's cheaper to just move them as
+        // it may avoid a malloc/realloc. Note that there's no technical reason
+        // for using this particular value, but it works well in real-world use
+        // cases.
+        static const size_t MaxElementsToMoveEagerly = 20;
+
+        if (getElementsHeader()->initializedLength <= MaxElementsToMoveEagerly)
+            moveShiftedElements();
+        else
+            maybeMoveShiftedElements();
         if (getDenseCapacity() >= reqCapacity)
             return true;
         numShifted = getElementsHeader()->numShiftedElements();
 
-        // Ensure |reqCapacity + numShifted| below won't overflow by forcing an
-        // unshift in that case.
+        // If |reqCapacity + numShifted| overflows, we just move all shifted
+        // elements to avoid the problem.
         CheckedInt<uint32_t> checkedReqCapacity(reqCapacity);
         checkedReqCapacity += numShifted;
         if (MOZ_UNLIKELY(!checkedReqCapacity.isValid())) {
-            unshiftElements();
+            moveShiftedElements();
             numShifted = 0;
         }
     }
@@ -895,10 +985,10 @@ NativeObject::shrinkElements(ExclusiveContext* cx, uint32_t reqCapacity)
     if (!hasDynamicElements())
         return;
 
-    // If we have shifted elements, consider unshifting them.
+    // If we have shifted elements, consider moving them.
     uint32_t numShifted = getElementsHeader()->numShiftedElements();
     if (numShifted > 0) {
-        maybeUnshiftElements();
+        maybeMoveShiftedElements();
         numShifted = getElementsHeader()->numShiftedElements();
     }
 

js/src/vm/NativeObject.h

@@ -169,7 +169,7 @@ ArraySetLength(JSContext* cx, Handle<ArrayObject*> obj, HandleId id,
  * to the next element and moving the ObjectElements header in memory (so it's
  * stored where the shifted Value used to be).
  *
- * Shifted elements can be unshifted when we grow the array, when the array is
+ * Shifted elements can be moved when we grow the array, when the array is
  * frozen (for simplicity, shifted elements are not supported on objects that
  * are frozen, have copy-on-write elements, or on arrays with non-writable
  * length).
@@ -205,12 +205,13 @@ class ObjectElements
     };
 
     // The flags word stores both the flags and the number of shifted elements.
-    // Allow shifting 2047 elements before unshifting.
-    static const size_t NumShiftedElementsBits = 11;
+    // Allow shifting 1023 elements before actually moving the elements.
+    // (Reduced from 2047 due to memory concerns on our 32-bit Power Macs.)
+    static const size_t NumShiftedElementsBits = 10;
     static const size_t MaxShiftedElements = (1 << NumShiftedElementsBits) - 1;
     static const size_t NumShiftedElementsShift = 32 - NumShiftedElementsBits;
     static const size_t FlagsMask = (1 << NumShiftedElementsShift) - 1;
-    static_assert(MaxShiftedElements == 2047,
+    static_assert(MaxShiftedElements == 1023,
                   "MaxShiftedElements should match the comment");
 
   private:
@@ -280,6 +281,16 @@ class ObjectElements
         capacity -= count;
         initializedLength -= count;
     }
+    void unshiftShiftedElements(uint32_t count) {
+        MOZ_ASSERT(count > 0);
+        MOZ_ASSERT(!(flags & (NONWRITABLE_ARRAY_LENGTH | FROZEN | COPY_ON_WRITE)));
+        uint32_t numShifted = numShiftedElements();
+        MOZ_ASSERT(count <= numShifted);
+        numShifted -= count;
+        flags = (numShifted << NumShiftedElementsShift) | (flags & FlagsMask);
+        capacity += count;
+        initializedLength += count;
+    }
     void clearShiftedElements() {
         flags &= FlagsMask;
         MOZ_ASSERT(numShiftedElements() == 0);
@@ -917,6 +928,8 @@ class NativeObject : public JSObject
             elements_[i].HeapSlot::~HeapSlot();
     }
 
+    inline void shiftDenseElementsUnchecked(uint32_t count);
+
     static bool rollbackProperties(ExclusiveContext* cx, HandleNativeObject obj,
                                    uint32_t slotSpan);
 
@@ -1032,11 +1045,15 @@ class NativeObject : public JSObject
     // Try to shift |count| dense elements, see the "Shifted elements" comment.
     inline bool tryShiftDenseElements(uint32_t count);
 
-    // Unshift all shifted elements so that numShiftedElements is 0.
-    void unshiftElements();
+    // Try to make space for |count| dense elements at the start of the array.
+    bool tryUnshiftDenseElements(uint32_t count);
 
-    // If this object has many shifted elements, unshift them.
-    void maybeUnshiftElements();
+    // Move the elements header and all shifted elements to the start of the
+    // allocated elements space, so that numShiftedElements is 0 afterwards.
+    void moveShiftedElements();
+
+    // If this object has many shifted elements call moveShiftedElements.
+    void maybeMoveShiftedElements();
 
     static bool goodElementsAllocationAmount(ExclusiveContext* cx, uint32_t reqAllocated,
                                              uint32_t length, uint32_t* goodAmount);