Bug 1364346 part 2 - Rename unshiftElements to moveShiftedElements, tweak heuristics. r=anba

js/src/vm/ArrayObject.h

@@ -33,7 +33,7 @@ class ArrayObject : public NativeObject
 
     void setNonWritableLength(JSContext* cx) {
         if (getElementsHeader()->numShiftedElements() > 0)
-            unshiftElements();
+            moveShiftedElements();
 
         // When an array's length becomes non-writable, writes to indexes
         // greater than or equal to the length don't change the array.  We

js/src/vm/NativeObject-inl.h

@@ -185,7 +185,7 @@ NativeObject::tryShiftDenseElements(uint32_t count)
     MOZ_ASSERT(count < header->initializedLength);
 
     if (MOZ_UNLIKELY(header->numShiftedElements() + count > ObjectElements::MaxShiftedElements)) {
-        unshiftElements();
+        moveShiftedElements();
         header = getElementsHeader();
     }
 

js/src/vm/NativeObject.cpp

@@ -115,7 +115,7 @@ ObjectElements::FreezeElements(JSContext* cx, HandleNativeObject obj)
         return true;
 
     if (obj->getElementsHeader()->numShiftedElements() > 0)
-        obj->unshiftElements();
+        obj->moveShiftedElements();
 
     ObjectElements* header = obj->getElementsHeader();
 
@@ -707,7 +707,7 @@ NativeObject::maybeDensifySparseElements(JSContext* cx, HandleNativeObject obj)
 }
 
 void
-NativeObject::unshiftElements()
+NativeObject::moveShiftedElements()
 {
     ObjectElements* header = getElementsHeader();
     uint32_t numShifted = header->numShiftedElements();
@@ -723,7 +723,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
@@ -739,14 +739,14 @@ 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();
 }
 
 // Given a requested capacity (in elements) and (potentially) the length of an
@@ -847,22 +847,31 @@ NativeObject::growElements(JSContext* 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;
         }
     }
@@ -932,10 +941,10 @@ NativeObject::shrinkElements(JSContext* 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

@@ -170,7 +170,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,7 +205,7 @@ class ObjectElements
     };
 
     // The flags word stores both the flags and the number of shifted elements.
-    // Allow shifting 2047 elements before unshifting.
+    // Allow shifting 2047 elements before actually moving the elements.
     static const size_t NumShiftedElementsBits = 11;
     static const size_t MaxShiftedElements = (1 << NumShiftedElementsBits) - 1;
     static const size_t NumShiftedElementsShift = 32 - NumShiftedElementsBits;
@@ -1074,11 +1074,12 @@ class NativeObject : public ShapedObject
     // 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();
+    // 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, unshift them.
-    void maybeUnshiftElements();
+    // If this object has many shifted elements call moveShiftedElements.
+    void maybeMoveShiftedElements();
 
     static bool goodElementsAllocationAmount(JSContext* cx, uint32_t reqAllocated,
                                              uint32_t length, uint32_t* goodAmount);