1344173 - Fix performance cliff involving OBJECT_FLAG_ITERATED and array natives also includes 1323782.

1344173 - Fix performance cliff involving OBJECT_FLAG_ITERATED and array natives.

1323782: Check iterated-flag before proceeding to fast path in Array.prototype.unshift.

js/src/jsarray.cpp

@@ -526,6 +526,36 @@ struct ReverseIndexComparator
     }
 };
 
+static bool
+MaybeInIteration(HandleObject obj, JSContext* cx)
+{
+    /*
+     * Don't optimize if the array might be in the midst of iteration.  We
+     * rely on this to be able to safely move dense array elements around with
+     * just a memmove (see NativeObject::moveDenseArrayElements), without worrying
+     * about updating any in-progress enumerators for properties implicitly
+     * deleted if a hole is moved from one location to another location not yet
+     * visited.  See bug 690622.
+     *
+     * Note that it's fine to optimize if |obj| is on the prototype of another
+     * object: SuppressDeletedProperty only suppresses properties deleted from
+     * the iterated object itself.
+     */
+
+    if (MOZ_LIKELY(!cx->compartment()->objectMaybeInIteration(obj)))
+        return false;
+
+    ObjectGroup* group = JSObject::getGroup(cx, obj);
+    if (MOZ_UNLIKELY(!group)) {
+        cx->recoverFromOutOfMemory();
+        return true;
+    }
+
+    if (MOZ_UNLIKELY(group->hasAllFlags(OBJECT_FLAG_ITERATED)))
+        return true;
+
+    return false;
+}
 bool
 js::CanonicalizeArrayLengthValue(JSContext* cx, HandleValue v, uint32_t* newLen)
 {
@@ -626,10 +656,7 @@ js::ArraySetLength(JSContext* cx, Handle<ArrayObject*> arr, HandleId id,
         // for..in iteration over the array. Keys deleted before being reached
         // during the iteration must not be visited, and suppressing them here
         // would be too costly.
-        ObjectGroup* arrGroup = JSObject::getGroup(cx, arr);
-        if (MOZ_UNLIKELY(!arrGroup))
-            return false;
-        if (!arr->isIndexed() && !MOZ_UNLIKELY(arrGroup->hasAllFlags(OBJECT_FLAG_ITERATED))) {
+        if (!arr->isIndexed() && !MaybeInIteration(arr, cx)) {
             if (!arr->maybeCopyElementsForWrite(cx))
                 return false;
 
@@ -2221,11 +2248,7 @@ ArrayShiftDenseKernel(JSContext* cx, HandleObject obj, MutableHandleValue rval)
     if (ObjectMayHaveExtraIndexedProperties(obj))
         return DenseElementResult::Incomplete;
 
-    RootedObjectGroup group(cx, JSObject::getGroup(cx, obj));
-    if (MOZ_UNLIKELY(!group))
-        return DenseElementResult::Failure;
-
-    if (MOZ_UNLIKELY(group->hasAllFlags(OBJECT_FLAG_ITERATED)))
+    if (MaybeInIteration(obj, cx))
         return DenseElementResult::Incomplete;
 
     size_t initlen = GetBoxedOrUnboxedInitializedLength<Type>(obj);
@@ -2331,22 +2354,28 @@ js::array_unshift(JSContext* cx, unsigned argc, Value* vp)
 
     const unsigned argCount = args.length();
     double newlen = length;
-    if (argCount > 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
-            // be optimized here because unshifting temporarily places holes at
-            // the start of the array.
-            bool optimized = false;
-            do {
-                if (!obj->is<ArrayObject>())
-                    break;
-                if (ObjectMayHaveExtraIndexedProperties(obj))
-                    break;
-                ArrayObject* aobj = &obj->as<ArrayObject>();
-                if (!aobj->lengthIsWritable())
-                    break;
-                DenseElementResult result = aobj->ensureDenseElements(cx, length, argCount);
+    if (args.length() > 0) {
+        // Only include a fast path for native objects. Unboxed arrays can't
+        // be optimized here because unshifting temporarily places holes at
+        // the start of the array.
+        // TODO: Implement unboxed array optimization similar to the one in
+        // array_splice_impl(), unshift() is a special version of splice():
+        // arr.unshift(...values) ~= arr.splice(0, 0, ...values).
+        bool optimized = false;
+        do {
+            if (!obj->isNative())
+                break;
+            if (ObjectMayHaveExtraIndexedProperties(obj))
+                break;
+            if (MaybeInIteration(obj, cx))
+                break;
+            NativeObject* nobj = &obj->as<NativeObject>();
+            if (nobj->denseElementsAreFrozen())
+                break;
+            if (nobj->is<ArrayObject>() && !nobj->as<ArrayObject>().lengthIsWritable())
+                break;
+            if (!nobj->tryUnshiftDenseElements(args.length())) {
+                DenseElementResult result = nobj->ensureDenseElements(cx, length, args.length());
                 if (result != DenseElementResult::Success) {
                     if (result == DenseElementResult::Failure)
                         return false;
@@ -2402,6 +2431,7 @@ js::array_unshift(JSContext* cx, unsigned argc, Value* vp)
  * etc. along the prototype chain, or of enumerators requiring notification of
  * modifications.
  */
+
 static inline bool
 CanOptimizeForDenseStorage(HandleObject arr, uint32_t startingIndex, uint32_t count, JSContext* cx)
 {
@@ -2417,27 +2447,8 @@ CanOptimizeForDenseStorage(HandleObject arr, uint32_t startingIndex, uint32_t co
     if (arr->is<ArrayObject>() && arr->as<ArrayObject>().denseElementsAreFrozen())
         return false;
 
-    /*
-     * Don't optimize if the array might be in the midst of iteration.  We
-     * rely on this to be able to safely move dense array elements around with
-     * just a memmove (see NativeObject::moveDenseArrayElements), without worrying
-     * about updating any in-progress enumerators for properties implicitly
-     * deleted if a hole is moved from one location to another location not yet
-     * visited.  See bug 690622.
-     */
-    ObjectGroup* arrGroup = JSObject::getGroup(cx, arr);
-    if (!arrGroup) {
-        cx->recoverFromOutOfMemory();
-        return false;
-    }
-    if (MOZ_UNLIKELY(arrGroup->hasAllFlags(OBJECT_FLAG_ITERATED)))
-        return false;
-
-    /*
-     * Another potential wrinkle: what if the enumeration is happening on an
-     * object which merely has |arr| on its prototype chain?
-     */
-    if (arr->isDelegate())
+    /* Also pick the slow path if the object is being iterated over. */
+    if (MaybeInIteration(arr, cx))
         return false;
 
     /*

js/src/jscompartment.h

@@ -852,6 +852,7 @@ struct JSCompartment
         compartmentStats_ = newStats;
     }
 
+    MOZ_ALWAYS_INLINE bool objectMaybeInIteration(JSObject* obj);
     // These flags help us to discover if a compartment that shouldn't be alive
     // manages to outlive a GC.
     bool scheduledForDestruction;

js/src/jscompartmentinlines.h

@@ -7,6 +7,7 @@
 #define jscompartmentinlines_h
 
 #include "jscompartment.h"
+#include "jsiter.h"
 
 #include "gc/Barrier.h"
 
@@ -130,4 +131,21 @@ JSCompartment::wrap(JSContext* cx, JS::MutableHandleValue vp)
     return true;
 }
 
+MOZ_ALWAYS_INLINE bool
+JSCompartment::objectMaybeInIteration(JSObject* obj)
+{
+    MOZ_ASSERT(obj->compartment() == this);
+
+    // If the list is empty we're not iterating any objects.
+    js::NativeIterator* next = enumerators->next();
+    if (enumerators == next)
+        return false;
+
+    // If the list contains a single object, check if it's |obj|.
+    if (next->next() == enumerators)
+        return next->obj == obj;
+
+    return true;
+}
+
 #endif /* jscompartmentinlines_h */

js/src/jsiter.cpp

@@ -1500,6 +1500,9 @@ public:
 bool
 js::SuppressDeletedProperty(JSContext* cx, HandleObject obj, jsid id)
 {
+    if (MOZ_LIKELY(!cx->compartment()->objectMaybeInIteration(obj)))
+        return true;
+
     if (JSID_IS_SYMBOL(id))
         return true;
 
@@ -1512,10 +1515,16 @@ js::SuppressDeletedProperty(JSContext* cx, HandleObject obj, jsid id)
 bool
 js::SuppressDeletedElement(JSContext* cx, HandleObject obj, uint32_t index)
 {
+    if (MOZ_LIKELY(!cx->compartment()->objectMaybeInIteration(obj)))
+        return true;
     RootedId id(cx);
     if (!IndexToId(cx, index, &id))
         return false;
-    return SuppressDeletedProperty(cx, obj, id);
+
+    Rooted<JSFlatString*> str(cx, IdToString(cx, id));
+    if (!str)
+        return false;
+    return SuppressDeletedPropertyHelper(cx, obj, SingleStringPredicate(str));
 }
 
 bool