Bug 1312485: array: Don't optimize for dense storage if the elements are frozen. r=nbp
authorEmilio Cobos Álvarez <ecoal95@gmail.com>
Mon, 24 Oct 2016 22:37:48 +0200
changeset 319755 b46310f1fb77fc686302b10e11f15634bab7844a
parent 319754 0bab744bba82fed941beadf4416ef01c30f54abf
child 319756 e97f6b0391f98d675ce930412838f91c8b40e21a
push id20748
push userphilringnalda@gmail.com
push dateFri, 28 Oct 2016 03:39:55 +0000
treeherderfx-team@715360440695 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp
bugs1312485
milestone52.0a1
Bug 1312485: array: Don't optimize for dense storage if the elements are frozen. r=nbp We're going to throw right away in most cases anyway. MozReview-Commit-ID: 7YxAybEP1UQ
js/src/jsarray.cpp
js/src/tests/ecma_5/Array/frozen-dense-array.js
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -2377,16 +2377,20 @@ CanOptimizeForDenseStorage(HandleObject 
     /* 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 an array. */
     if (!arr->is<ArrayObject>() && !arr->is<UnboxedArrayObject>())
         return false;
 
+    /* If it's a frozen array, always pick the slow path */
+    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.
      */
--- a/js/src/tests/ecma_5/Array/frozen-dense-array.js
+++ b/js/src/tests/ecma_5/Array/frozen-dense-array.js
@@ -20,16 +20,17 @@ assertThrowsInstanceOf(() => a.shift(), 
 assertThrowsInstanceOf(() => a.unshift(0), TypeError);
 assertThrowsInstanceOf(() => a.sort(function() {}), TypeError);
 assertThrowsInstanceOf(() => a.pop(), TypeError);
 assertThrowsInstanceOf(() => a.fill(0), TypeError);
 assertThrowsInstanceOf(() => a.splice(0, 1, 1), TypeError);
 assertThrowsInstanceOf(() => a.push("foo"), TypeError);
 assertThrowsInstanceOf(() => { "use strict"; a.length = 5; }, TypeError);
 assertThrowsInstanceOf(() => { "use strict"; delete a[0]; }, TypeError);
+assertThrowsInstanceOf(() => a.splice(Math.a), TypeError);
 
 // Shouldn't throw, since this is not strict mode, but shouldn't change the
 // value of the property.
 a.length = 5;
 assertEq(delete a[0], false);
 
 
 assertEq(a.length, 3);