8138632: Sparse array does not handle growth of underlying dense array
authorhannesw
Thu, 01 Oct 2015 10:37:25 +0200
changeset 32892 7186b06a72cf
parent 32891 687132a414b2
child 32893 665eb8283882
8138632: Sparse array does not handle growth of underlying dense array Reviewed-by: attila, sundar
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/ArrayData.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/ByteBufferArrayData.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/SealedArrayFilter.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/SparseArrayData.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/TypedArrayData.java
nashorn/test/script/basic/JDK-8138632.js
nashorn/test/script/basic/JDK-8138632.js.EXPECTED
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java	Wed Sep 30 20:20:11 2015 +0530
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java	Thu Oct 01 10:37:25 2015 +0200
@@ -710,8 +710,7 @@
         final long longIndex = ArrayIndex.toLongIndex(index);
         final long oldLength = getArray().length();
         if (longIndex >= oldLength) {
-            setArray(getArray().ensure(longIndex));
-            doesNotHaveEnsureDelete(longIndex, oldLength, false);
+            setArray(getArray().ensure(longIndex).safeDelete(oldLength, longIndex - 1, false));
         }
         setArray(getArray().set(index, value, false));
     }
@@ -2693,11 +2692,7 @@
         }
 
         if (newLength > arrayLength) {
-            data = data.ensure(newLength - 1);
-            if (data.canDelete(arrayLength, newLength - 1, false)) {
-               data = data.delete(arrayLength, newLength - 1);
-            }
-            setArray(data);
+            setArray(data.ensure(newLength - 1).safeDelete(arrayLength, newLength - 1, false));
             return;
         }
 
@@ -3118,23 +3113,12 @@
         return false;
     }
 
-    private void doesNotHaveEnsureDelete(final long longIndex, final long oldLength, final boolean strict) {
-        if (longIndex > oldLength) {
-            ArrayData array = getArray();
-            if (array.canDelete(oldLength, longIndex - 1, strict)) {
-                array = array.delete(oldLength, longIndex - 1);
-            }
-            setArray(array);
-        }
-    }
-
     private void doesNotHave(final int index, final int value, final int callSiteFlags) {
         final long oldLength = getArray().length();
         final long longIndex = ArrayIndex.toLongIndex(index);
         if (!doesNotHaveCheckArrayKeys(longIndex, value, callSiteFlags) && !doesNotHaveEnsureLength(longIndex, oldLength, callSiteFlags)) {
             final boolean strict = isStrictFlag(callSiteFlags);
-            setArray(getArray().set(index, value, strict));
-            doesNotHaveEnsureDelete(longIndex, oldLength, strict);
+            setArray(getArray().set(index, value, strict).safeDelete(oldLength, longIndex - 1, strict));
         }
     }
 
@@ -3143,8 +3127,7 @@
         final long longIndex = ArrayIndex.toLongIndex(index);
         if (!doesNotHaveCheckArrayKeys(longIndex, value, callSiteFlags) && !doesNotHaveEnsureLength(longIndex, oldLength, callSiteFlags)) {
             final boolean strict = isStrictFlag(callSiteFlags);
-            setArray(getArray().set(index, value, strict));
-            doesNotHaveEnsureDelete(longIndex, oldLength, strict);
+            setArray(getArray().set(index, value, strict).safeDelete(oldLength, longIndex - 1, strict));
         }
     }
 
@@ -3153,8 +3136,7 @@
         final long longIndex = ArrayIndex.toLongIndex(index);
         if (!doesNotHaveCheckArrayKeys(longIndex, value, callSiteFlags) && !doesNotHaveEnsureLength(longIndex, oldLength, callSiteFlags)) {
             final boolean strict = isStrictFlag(callSiteFlags);
-            setArray(getArray().set(index, value, strict));
-            doesNotHaveEnsureDelete(longIndex, oldLength, strict);
+            setArray(getArray().set(index, value, strict).safeDelete(oldLength, longIndex - 1, strict));
         }
     }
 
@@ -3163,8 +3145,7 @@
         final long longIndex = ArrayIndex.toLongIndex(index);
         if (!doesNotHaveCheckArrayKeys(longIndex, value, callSiteFlags) && !doesNotHaveEnsureLength(longIndex, oldLength, callSiteFlags)) {
             final boolean strict = isStrictFlag(callSiteFlags);
-            setArray(getArray().set(index, value, strict));
-            doesNotHaveEnsureDelete(longIndex, oldLength, strict);
+            setArray(getArray().set(index, value, strict).safeDelete(oldLength, longIndex - 1, strict));
         }
     }
 
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/ArrayData.java	Wed Sep 30 20:20:11 2015 +0530
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/ArrayData.java	Thu Oct 01 10:37:25 2015 +0200
@@ -258,7 +258,7 @@
      * Factory method for unspecified array - start as int
      * @return ArrayData
      */
-    public final static ArrayData initialArray() {
+    public static ArrayData initialArray() {
         return new IntArrayData();
     }
 
@@ -278,7 +278,7 @@
      * @param size size required
      * @return size given, always >= size
      */
-    protected final static int alignUp(final int size) {
+    protected static int alignUp(final int size) {
         return size + CHUNK_SIZE - 1 & ~(CHUNK_SIZE - 1);
     }
 
@@ -288,7 +288,7 @@
      * @param length the initial length
      * @return ArrayData
      */
-    public static final ArrayData allocate(final int length) {
+    public static ArrayData allocate(final int length) {
         if (length == 0) {
             return new IntArrayData();
         } else if (length >= SparseArrayData.MAX_DENSE_LENGTH) {
@@ -304,7 +304,7 @@
      * @param  array the array
      * @return ArrayData wrapping this array
      */
-    public static final ArrayData allocate(final Object array) {
+    public static ArrayData allocate(final Object array) {
         final Class<?> clazz = array.getClass();
 
         if (clazz == int[].class) {
@@ -324,7 +324,7 @@
      * @param array the array to use for initial elements
      * @return the ArrayData
      */
-    public static final ArrayData allocate(final int[] array) {
+    public static ArrayData allocate(final int[] array) {
          return new IntArrayData(array, array.length);
     }
 
@@ -334,7 +334,7 @@
      * @param array the array to use for initial elements
      * @return the ArrayData
      */
-    public static final ArrayData allocate(final long[] array) {
+    public static ArrayData allocate(final long[] array) {
         return new LongArrayData(array, array.length);
     }
 
@@ -344,7 +344,7 @@
      * @param array the array to use for initial elements
      * @return the ArrayData
      */
-    public static final ArrayData allocate(final double[] array) {
+    public static ArrayData allocate(final double[] array) {
         return new NumberArrayData(array, array.length);
     }
 
@@ -354,7 +354,7 @@
      * @param array the array to use for initial elements
      * @return the ArrayData
      */
-    public static final ArrayData allocate(final Object[] array) {
+    public static ArrayData allocate(final Object[] array) {
         return new ObjectArrayData(array, array.length);
     }
 
@@ -364,7 +364,7 @@
      * @param buf the nio ByteBuffer to wrap
      * @return the ArrayData
      */
-    public static final ArrayData allocate(final ByteBuffer buf) {
+    public static ArrayData allocate(final ByteBuffer buf) {
         return new ByteBufferArrayData(buf);
     }
 
@@ -374,7 +374,7 @@
      * @param underlying  the underlying ArrayData to wrap in the freeze filter
      * @return the frozen ArrayData
      */
-    public static final ArrayData freeze(final ArrayData underlying) {
+    public static ArrayData freeze(final ArrayData underlying) {
         return new FrozenArrayFilter(underlying);
     }
 
@@ -384,7 +384,7 @@
      * @param underlying  the underlying ArrayData to wrap in the seal filter
      * @return the sealed ArrayData
      */
-    public static final ArrayData seal(final ArrayData underlying) {
+    public static ArrayData seal(final ArrayData underlying) {
         return new SealedArrayFilter(underlying);
     }
 
@@ -394,7 +394,7 @@
      * @param  underlying the underlying ArrayData to wrap in the non extensible filter
      * @return new array data, filtered
      */
-    public static final ArrayData preventExtension(final ArrayData underlying) {
+    public static ArrayData preventExtension(final ArrayData underlying) {
         return new NonExtensibleArrayFilter(underlying);
     }
 
@@ -404,7 +404,7 @@
      * @param underlying the underlying ArrayDAta to wrap in the non extensible filter
      * @return new array data, filtered
      */
-    public static final ArrayData setIsLengthNotWritable(final ArrayData underlying) {
+    public static ArrayData setIsLengthNotWritable(final ArrayData underlying) {
         return new LengthNotWritableFilter(underlying);
     }
 
@@ -676,19 +676,34 @@
     }
 
     /**
-     * Returns if element at specific index range can be deleted or not.
+     * Returns if element at specific index can be deleted or not.
      *
-     * @param fromIndex  the start index
-     * @param toIndex    the end index
+     * @param longIndex  the index
      * @param strict     are we in strict mode
      *
      * @return true if range can be deleted
      */
-    public boolean canDelete(final long fromIndex, final long toIndex, final boolean strict) {
+    public boolean canDelete(final long longIndex, final boolean strict) {
         return true;
     }
 
     /**
+     * Delete a range from the array if {@code fromIndex} is less than or equal to {@code toIndex}
+     * and the array supports deletion.
+     *
+     * @param fromIndex  the start index (inclusive)
+     * @param toIndex    the end index (inclusive)
+     * @param strict     are we in strict mode
+     * @return an array with the range deleted, or this array if no deletion took place
+     */
+    public final ArrayData safeDelete(final long fromIndex, final long toIndex, final boolean strict) {
+        if (fromIndex <= toIndex && canDelete(fromIndex, strict)) {
+            return delete(fromIndex, toIndex);
+        }
+        return this;
+    }
+
+    /**
      * Returns property descriptor for element at a given index
      *
      * @param global the global object
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/ByteBufferArrayData.java	Wed Sep 30 20:20:11 2015 +0530
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/ByteBufferArrayData.java	Thu Oct 01 10:37:25 2015 +0200
@@ -164,7 +164,7 @@
     }
 
     @Override
-    public boolean canDelete(final long fromIndex, final long toIndex, final boolean strict) {
+    public boolean canDelete(final long longIndex, final boolean strict) {
         return false;
     }
 
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/SealedArrayFilter.java	Wed Sep 30 20:20:11 2015 +0530
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/SealedArrayFilter.java	Thu Oct 01 10:37:25 2015 +0200
@@ -50,15 +50,15 @@
 
     @Override
     public boolean canDelete(final int index, final boolean strict) {
-        if (strict) {
-            throw typeError("cant.delete.property", Integer.toString(index), "sealed array");
-        }
-        return false;
+        return canDelete(ArrayIndex.toLongIndex(index), strict);
     }
 
     @Override
-    public boolean canDelete(final long fromIndex, final long toIndex, final boolean strict) {
-        return canDelete((int) fromIndex, strict);
+    public boolean canDelete(final long longIndex, final boolean strict) {
+        if (strict) {
+            throw typeError("cant.delete.property", Long.toString(longIndex), "sealed array");
+        }
+        return false;
     }
 
     @Override
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/SparseArrayData.java	Wed Sep 30 20:20:11 2015 +0530
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/SparseArrayData.java	Thu Oct 01 10:37:25 2015 +0200
@@ -166,8 +166,9 @@
     @Override
     public ArrayData set(final int index, final Object value, final boolean strict) {
         if (index >= 0 && index < maxDenseLength) {
+            final long oldLength = underlying.length();
             ensure(index);
-            underlying = underlying.set(index, value, strict);
+            underlying = underlying.set(index, value, strict).safeDelete(oldLength, index - 1, strict);
             setLength(Math.max(underlying.length(), length()));
         } else {
             final Long longIndex = indexToKey(index);
@@ -181,8 +182,9 @@
     @Override
     public ArrayData set(final int index, final int value, final boolean strict) {
         if (index >= 0 && index < maxDenseLength) {
+            final long oldLength = underlying.length();
             ensure(index);
-            underlying = underlying.set(index, value, strict);
+            underlying = underlying.set(index, value, strict).safeDelete(oldLength, index - 1, strict);
             setLength(Math.max(underlying.length(), length()));
         } else {
             final Long longIndex = indexToKey(index);
@@ -195,8 +197,9 @@
     @Override
     public ArrayData set(final int index, final long value, final boolean strict) {
         if (index >= 0 && index < maxDenseLength) {
+            final long oldLength = underlying.length();
             ensure(index);
-            underlying = underlying.set(index, value, strict);
+            underlying = underlying.set(index, value, strict).safeDelete(oldLength, index - 1, strict);
             setLength(Math.max(underlying.length(), length()));
         } else {
             final Long longIndex = indexToKey(index);
@@ -209,8 +212,9 @@
     @Override
     public ArrayData set(final int index, final double value, final boolean strict) {
         if (index >= 0 && index < maxDenseLength) {
+            final long oldLength = underlying.length();
             ensure(index);
-            underlying = underlying.set(index, value, strict);
+            underlying = underlying.set(index, value, strict).safeDelete(oldLength, index - 1, strict);
             setLength(Math.max(underlying.length(), length()));
         } else {
             final Long longIndex = indexToKey(index);
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/TypedArrayData.java	Wed Sep 30 20:20:11 2015 +0530
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/arrays/TypedArrayData.java	Thu Oct 01 10:37:25 2015 +0200
@@ -83,7 +83,7 @@
     }
 
     @Override
-    public boolean canDelete(final long fromIndex, final long toIndex, final boolean strict) {
+    public boolean canDelete(final long longIndex, final boolean strict) {
         return false;
     }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8138632.js	Thu Oct 01 10:37:25 2015 +0200
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ * 
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ * 
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ * 
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ * 
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8138632: Sparse array does not handle growth of underlying dense array
+ *
+ * @test
+ * @run
+ */
+
+var x = [];
+x[10000000] = 1;
+x[10] = 1;
+x[20] = 1;
+print(Object.keys(x));
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8138632.js.EXPECTED	Thu Oct 01 10:37:25 2015 +0200
@@ -0,0 +1,1 @@
+10,20,10000000