Bug 694210 - Fix regressions from bug 668024 landing: first a forgotten check for denseness exceeding 2**32 - 1, second for a performance regression from an optimization condition that doesn't hold often enough and bit kraken-stanford-crypto-sha256-iterative. r=bhackett
authorJeff Walden <jwalden@mit.edu>
Mon, 17 Oct 2011 11:10:43 -0700
changeset 79622 5b13e63313a8b76b47db2b1066d1c5597f384441
parent 79621 f864bf022b6923924e2951f575a51cbd4dea8de2
child 79623 bdd89e099e60a020e3b6a8be3bef42374bf7a8e6
push id506
push userclegnitto@mozilla.com
push dateWed, 09 Nov 2011 02:03:18 +0000
treeherdermozilla-aurora@63587fc7bb93 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs694210, 668024
milestone10.0a1
Bug 694210 - Fix regressions from bug 668024 landing: first a forgotten check for denseness exceeding 2**32 - 1, second for a performance regression from an optimization condition that doesn't hold often enough and bit kraken-stanford-crypto-sha256-iterative. r=bhackett
js/src/jsarray.cpp
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -2751,28 +2751,49 @@ TryReuseArrayType(JSObject *obj, JSObjec
     if (obj->isArray() && !obj->hasSingletonType() && obj->getProto() == nobj->getProto())
         nobj->setType(obj->type());
 }
 
 /*
  * Returns true if this is a dense array whose |count| properties starting from
  * |startingIndex| may be accessed (get, set, delete) directly through its
  * contiguous vector of elements without fear of getters, setters, etc. along
- * the prototype chain.
+ * the prototype chain, or of enumerators requiring notification of
+ * modifications.
  */
 static inline bool
 CanOptimizeForDenseStorage(JSObject *arr, uint32 startingIndex, uint32 count, JSContext *cx)
 {
-    JS_ASSERT(UINT32_MAX - startingIndex >= count);
-
-    uint32 length = startingIndex + count;
-    return arr->isDenseArray() &&
-           !arr->getType(cx)->hasAllFlags(OBJECT_FLAG_NON_PACKED_ARRAY) &&
-           !js_PrototypeHasIndexedProperties(cx, arr) &&
-           length <= arr->getDenseArrayInitializedLength();
+    /* If the desired properties overflow dense storage, we can't optimize. */
+    if (UINT32_MAX - startingIndex < count)
+        return false;
+
+    /* There's no optimizing possible if it's not a dense array. */
+    if (!arr->isDenseArray())
+        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 JSObject::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.
+     *
+     * Another potential wrinkle: what if the enumeration is happening on an
+     * object which merely has |arr| on its prototype chain?  It turns out this
+     * case can't happen, because any dense array used as the prototype of
+     * another object is first slowified, for type inference's sake.
+     */
+    if (JS_UNLIKELY(arr->getType(cx)->hasAllFlags(OBJECT_FLAG_ITERATED)))
+        return false;
+
+    /* Now just watch out for getters and setters along the prototype chain. */
+    return !js_PrototypeHasIndexedProperties(cx, arr) &&
+           startingIndex + count <= arr->getDenseArrayInitializedLength();
 }
 
 static inline bool
 CopyArrayElement(JSContext *cx, JSObject *source, uint32 sourceIndex,
                  JSObject *target, uint32 targetIndex)
 {
     if (!JS_CHECK_OPERATION_LIMIT(cx))
         return false;