6933217: Huge arrays handled poorly in core libraries
authormartin
Sun, 09 May 2010 00:59:30 -0700
changeset 5466 f130bb07764b
parent 5465 950789cfc3c5
child 5467 0cfa1c70b5ab
6933217: Huge arrays handled poorly in core libraries Summary: Write overflow-conscious array resizing code Reviewed-by: chegar
jdk/src/share/classes/java/io/ByteArrayOutputStream.java
jdk/src/share/classes/java/lang/AbstractStringBuilder.java
jdk/src/share/classes/java/util/AbstractCollection.java
jdk/src/share/classes/java/util/ArrayList.java
jdk/src/share/classes/java/util/Hashtable.java
jdk/src/share/classes/java/util/PriorityQueue.java
jdk/src/share/classes/java/util/Vector.java
--- a/jdk/src/share/classes/java/io/ByteArrayOutputStream.java	Fri May 07 16:11:13 2010 +0100
+++ b/jdk/src/share/classes/java/io/ByteArrayOutputStream.java	Sun May 09 00:59:30 2010 -0700
@@ -78,17 +78,50 @@
     }
 
     /**
+     * Increases the capacity if necessary to ensure that it can hold
+     * at least the number of elements specified by the minimum
+     * capacity argument.
+     *
+     * @param minCapacity the desired minimum capacity
+     * @throws OutOfMemoryError if {@code minCapacity < 0}.  This is
+     * interpreted as a request for the unsatisfiably large capacity
+     * {@code (long) Integer.MAX_VALUE + (minCapacity - Integer.MAX_VALUE)}.
+     */
+    private void ensureCapacity(int minCapacity) {
+        // overflow-conscious code
+        if (minCapacity - buf.length > 0)
+            grow(minCapacity);
+    }
+
+    /**
+     * Increases the capacity to ensure that it can hold at least the
+     * number of elements specified by the minimum capacity argument.
+     *
+     * @param minCapacity the desired minimum capacity
+     */
+    private void grow(int minCapacity) {
+        // overflow-conscious code
+        int oldCapacity = buf.length;
+        int newCapacity = oldCapacity << 1;
+        if (newCapacity - minCapacity < 0)
+            newCapacity = minCapacity;
+        if (newCapacity < 0) {
+            if (minCapacity < 0) // overflow
+                throw new OutOfMemoryError();
+            newCapacity = Integer.MAX_VALUE;
+        }
+        buf = Arrays.copyOf(buf, newCapacity);
+    }
+
+    /**
      * Writes the specified byte to this byte array output stream.
      *
      * @param   b   the byte to be written.
      */
     public synchronized void write(int b) {
-        int newcount = count + 1;
-        if (newcount > buf.length) {
-            buf = Arrays.copyOf(buf, Math.max(buf.length << 1, newcount));
-        }
-        buf[count] = (byte)b;
-        count = newcount;
+        ensureCapacity(count + 1);
+        buf[count] = (byte) b;
+        count += 1;
     }
 
     /**
@@ -101,17 +134,12 @@
      */
     public synchronized void write(byte b[], int off, int len) {
         if ((off < 0) || (off > b.length) || (len < 0) ||
-            ((off + len) > b.length) || ((off + len) < 0)) {
+            ((off + len) - b.length > 0)) {
             throw new IndexOutOfBoundsException();
-        } else if (len == 0) {
-            return;
         }
-        int newcount = count + len;
-        if (newcount > buf.length) {
-            buf = Arrays.copyOf(buf, Math.max(buf.length << 1, newcount));
-        }
+        ensureCapacity(count + len);
         System.arraycopy(b, off, buf, count, len);
-        count = newcount;
+        count += len;
     }
 
     /**
--- a/jdk/src/share/classes/java/lang/AbstractStringBuilder.java	Fri May 07 16:11:13 2010 +0100
+++ b/jdk/src/share/classes/java/lang/AbstractStringBuilder.java	Sun May 09 00:59:30 2010 -0700
@@ -36,6 +36,8 @@
  * sequence can be changed through certain method calls.
  *
  * @author      Michael McCloskey
+ * @author      Martin Buchholz
+ * @author      Ulf Zibis
  * @since       1.5
  */
 abstract class AbstractStringBuilder implements Appendable, CharSequence {
@@ -98,9 +100,16 @@
      * @param   minimumCapacity   the minimum desired capacity.
      */
     public void ensureCapacity(int minimumCapacity) {
-        if (minimumCapacity > value.length) {
+        ensureCapacityInternal(minimumCapacity);
+    }
+
+    /**
+     * This method has the same contract as ensureCapacity, but is
+     * never synchronized.
+     */
+    private void ensureCapacityInternal(int minimumCapacity) {
+        if (minimumCapacity - value.length > 0)
             expandCapacity(minimumCapacity);
-        }
     }
 
     /**
@@ -108,11 +117,13 @@
      * size check or synchronization.
      */
     void expandCapacity(int minimumCapacity) {
-        int newCapacity = (value.length + 1) * 2;
+        int newCapacity = value.length * 2;
+        if (newCapacity - minimumCapacity < 0)
+            newCapacity = minimumCapacity;
         if (newCapacity < 0) {
+            if (minimumCapacity < 0) // overflow
+                throw new OutOfMemoryError();
             newCapacity = Integer.MAX_VALUE;
-        } else if (minimumCapacity > newCapacity) {
-            newCapacity = minimumCapacity;
         }
         value = Arrays.copyOf(value, newCapacity);
     }
@@ -158,8 +169,7 @@
     public void setLength(int newLength) {
         if (newLength < 0)
             throw new StringIndexOutOfBoundsException(newLength);
-        if (newLength > value.length)
-            expandCapacity(newLength);
+        ensureCapacityInternal(newLength);
 
         if (count < newLength) {
             for (; count < newLength; count++)
@@ -400,12 +410,9 @@
     public AbstractStringBuilder append(String str) {
         if (str == null) str = "null";
         int len = str.length();
-        if (len == 0) return this;
-        int newCount = count + len;
-        if (newCount > value.length)
-            expandCapacity(newCount);
+        ensureCapacityInternal(count + len);
         str.getChars(0, len, value, count);
-        count = newCount;
+        count += len;
         return this;
     }
 
@@ -414,11 +421,9 @@
         if (sb == null)
             return append("null");
         int len = sb.length();
-        int newCount = count + len;
-        if (newCount > value.length)
-            expandCapacity(newCount);
+        ensureCapacityInternal(count + len);
         sb.getChars(0, len, value, count);
-        count = newCount;
+        count += len;
         return this;
     }
 
@@ -470,14 +475,10 @@
                 "start " + start + ", end " + end + ", s.length() "
                 + s.length());
         int len = end - start;
-        if (len == 0)
-            return this;
-        int newCount = count + len;
-        if (newCount > value.length)
-            expandCapacity(newCount);
-        for (int i=start; i<end; i++)
-            value[count++] = s.charAt(i);
-        count = newCount;
+        ensureCapacityInternal(count + len);
+        for (int i = start, j = count; i < end; i++, j++)
+            value[j] = s.charAt(i);
+        count += len;
         return this;
     }
 
@@ -498,11 +499,10 @@
      * @return  a reference to this object.
      */
     public AbstractStringBuilder append(char[] str) {
-        int newCount = count + str.length;
-        if (newCount > value.length)
-            expandCapacity(newCount);
-        System.arraycopy(str, 0, value, count, str.length);
-        count = newCount;
+        int len = str.length;
+        ensureCapacityInternal(count + len);
+        System.arraycopy(str, 0, value, count, len);
+        count += len;
         return this;
     }
 
@@ -529,11 +529,9 @@
      *         or {@code offset+len > str.length}
      */
     public AbstractStringBuilder append(char str[], int offset, int len) {
-        int newCount = count + len;
-        if (newCount > value.length)
-            expandCapacity(newCount);
+        ensureCapacityInternal(count + len);
         System.arraycopy(str, offset, value, count, len);
-        count = newCount;
+        count += len;
         return this;
     }
 
@@ -551,17 +549,13 @@
      */
     public AbstractStringBuilder append(boolean b) {
         if (b) {
-            int newCount = count + 4;
-            if (newCount > value.length)
-                expandCapacity(newCount);
+            ensureCapacityInternal(count + 4);
             value[count++] = 't';
             value[count++] = 'r';
             value[count++] = 'u';
             value[count++] = 'e';
         } else {
-            int newCount = count + 5;
-            if (newCount > value.length)
-                expandCapacity(newCount);
+            ensureCapacityInternal(count + 5);
             value[count++] = 'f';
             value[count++] = 'a';
             value[count++] = 'l';
@@ -587,9 +581,7 @@
      * @return  a reference to this object.
      */
     public AbstractStringBuilder append(char c) {
-        int newCount = count + 1;
-        if (newCount > value.length)
-            expandCapacity(newCount);
+        ensureCapacityInternal(count + 1);
         value[count++] = c;
         return this;
     }
@@ -614,8 +606,7 @@
         int appendedLength = (i < 0) ? Integer.stringSize(-i) + 1
                                      : Integer.stringSize(i);
         int spaceNeeded = count + appendedLength;
-        if (spaceNeeded > value.length)
-            expandCapacity(spaceNeeded);
+        ensureCapacityInternal(spaceNeeded);
         Integer.getChars(i, spaceNeeded, value);
         count = spaceNeeded;
         return this;
@@ -641,8 +632,7 @@
         int appendedLength = (l < 0) ? Long.stringSize(-l) + 1
                                      : Long.stringSize(l);
         int spaceNeeded = count + appendedLength;
-        if (spaceNeeded > value.length)
-            expandCapacity(spaceNeeded);
+        ensureCapacityInternal(spaceNeeded);
         Long.getChars(l, spaceNeeded, value);
         count = spaceNeeded;
         return this;
@@ -738,10 +728,7 @@
         if (codePoint >= Character.MIN_SUPPLEMENTARY_CODE_POINT) {
             n++;
         }
-        int newCount = count + n;
-        if (newCount > value.length) {
-            expandCapacity(newCount);
-        }
+        ensureCapacityInternal(count + n);
         if (n == 1) {
             value[count++] = (char) codePoint;
         } else {
@@ -807,8 +794,7 @@
             end = count;
         int len = str.length();
         int newCount = count + len - (end - start);
-        if (newCount > value.length)
-            expandCapacity(newCount);
+        ensureCapacityInternal(newCount);
 
         System.arraycopy(value, end, value, start + len, count - end);
         str.getChars(value, start);
@@ -915,12 +901,10 @@
             throw new StringIndexOutOfBoundsException(
                 "offset " + offset + ", len " + len + ", str.length "
                 + str.length);
-        int newCount = count + len;
-        if (newCount > value.length)
-            expandCapacity(newCount);
+        ensureCapacityInternal(count + len);
         System.arraycopy(value, index, value, index + len, count - index);
         System.arraycopy(str, offset, value, index, len);
-        count = newCount;
+        count += len;
         return this;
     }
 
@@ -984,12 +968,10 @@
         if (str == null)
             str = "null";
         int len = str.length();
-        int newCount = count + len;
-        if (newCount > value.length)
-            expandCapacity(newCount);
+        ensureCapacityInternal(count + len);
         System.arraycopy(value, offset, value, offset + len, count - offset);
         str.getChars(value, offset);
-        count = newCount;
+        count += len;
         return this;
     }
 
@@ -1021,12 +1003,10 @@
         if ((offset < 0) || (offset > length()))
             throw new StringIndexOutOfBoundsException(offset);
         int len = str.length;
-        int newCount = count + len;
-        if (newCount > value.length)
-            expandCapacity(newCount);
+        ensureCapacityInternal(count + len);
         System.arraycopy(value, offset, value, offset + len, count - offset);
         System.arraycopy(str, 0, value, offset, len);
-        count = newCount;
+        count += len;
         return this;
     }
 
@@ -1114,16 +1094,12 @@
                 "start " + start + ", end " + end + ", s.length() "
                 + s.length());
         int len = end - start;
-        if (len == 0)
-            return this;
-        int newCount = count + len;
-        if (newCount > value.length)
-            expandCapacity(newCount);
+        ensureCapacityInternal(count + len);
         System.arraycopy(value, dstOffset, value, dstOffset + len,
                          count - dstOffset);
         for (int i=start; i<end; i++)
             value[dstOffset++] = s.charAt(i);
-        count = newCount;
+        count += len;
         return this;
     }
 
@@ -1170,12 +1146,10 @@
      * @throws     IndexOutOfBoundsException  if the offset is invalid.
      */
     public AbstractStringBuilder insert(int offset, char c) {
-        int newCount = count + 1;
-        if (newCount > value.length)
-            expandCapacity(newCount);
+        ensureCapacityInternal(count + 1);
         System.arraycopy(value, offset, value, offset + 1, count - offset);
         value[offset] = c;
-        count = newCount;
+        count += 1;
         return this;
     }
 
--- a/jdk/src/share/classes/java/util/AbstractCollection.java	Fri May 07 16:11:13 2010 +0100
+++ b/jdk/src/share/classes/java/util/AbstractCollection.java	Sun May 09 00:59:30 2010 -0700
@@ -191,6 +191,14 @@
     }
 
     /**
+     * The maximum size of array to allocate.
+     * Some VMs reserve some header words in an array.
+     * Attempts to allocate larger arrays may result in
+     * OutOfMemoryError: Requested array size exceeds VM limit
+     */
+    private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8;
+
+    /**
      * Reallocates the array being used within toArray when the iterator
      * returned more elements than expected, and finishes filling it from
      * the iterator.
@@ -205,13 +213,10 @@
         while (it.hasNext()) {
             int cap = r.length;
             if (i == cap) {
-                int newCap = ((cap / 2) + 1) * 3;
-                if (newCap <= cap) { // integer overflow
-                    if (cap == Integer.MAX_VALUE)
-                        throw new OutOfMemoryError
-                            ("Required array size too large");
-                    newCap = Integer.MAX_VALUE;
-                }
+                int newCap = cap + (cap >> 1) + 1;
+                // overflow-conscious code
+                if (newCap - MAX_ARRAY_SIZE > 0)
+                    newCap = hugeCapacity(cap + 1);
                 r = Arrays.copyOf(r, newCap);
             }
             r[i++] = (T)it.next();
@@ -220,6 +225,15 @@
         return (i == r.length) ? r : Arrays.copyOf(r, i);
     }
 
+    private static int hugeCapacity(int minCapacity) {
+        if (minCapacity < 0) // overflow
+            throw new OutOfMemoryError
+                ("Required array size too large");
+        return (minCapacity > MAX_ARRAY_SIZE) ?
+            Integer.MAX_VALUE :
+            MAX_ARRAY_SIZE;
+    }
+
     // Modification Operations
 
     /**
--- a/jdk/src/share/classes/java/util/ArrayList.java	Fri May 07 16:11:13 2010 +0100
+++ b/jdk/src/share/classes/java/util/ArrayList.java	Sun May 09 00:59:30 2010 -0700
@@ -173,18 +173,47 @@
      * necessary, to ensure that it can hold at least the number of elements
      * specified by the minimum capacity argument.
      *
-     * @param   minCapacity   the desired minimum capacity
+     * @param minCapacity the desired minimum capacity
      */
     public void ensureCapacity(int minCapacity) {
         modCount++;
+        // overflow-conscious code
+        if (minCapacity - elementData.length > 0)
+            grow(minCapacity);
+    }
+
+    /**
+     * The maximum size of array to allocate.
+     * Some VMs reserve some header words in an array.
+     * Attempts to allocate larger arrays may result in
+     * OutOfMemoryError: Requested array size exceeds VM limit
+     */
+    private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8;
+
+    /**
+     * Increases the capacity to ensure that it can hold at least the
+     * number of elements specified by the minimum capacity argument.
+     *
+     * @param minCapacity the desired minimum capacity
+     */
+    private void grow(int minCapacity) {
+        // overflow-conscious code
         int oldCapacity = elementData.length;
-        if (minCapacity > oldCapacity) {
-            int newCapacity = (oldCapacity * 3)/2 + 1;
-            if (newCapacity < minCapacity)
-                newCapacity = minCapacity;
-            // minCapacity is usually close to size, so this is a win:
-            elementData = Arrays.copyOf(elementData, newCapacity);
-        }
+        int newCapacity = oldCapacity + (oldCapacity >> 1);
+        if (newCapacity - minCapacity < 0)
+            newCapacity = minCapacity;
+        if (newCapacity - MAX_ARRAY_SIZE > 0)
+            newCapacity = hugeCapacity(minCapacity);
+        // minCapacity is usually close to size, so this is a win:
+        elementData = Arrays.copyOf(elementData, newCapacity);
+    }
+
+    private static int hugeCapacity(int minCapacity) {
+        if (minCapacity < 0) // overflow
+            throw new OutOfMemoryError();
+        return (minCapacity > MAX_ARRAY_SIZE) ?
+            Integer.MAX_VALUE :
+            MAX_ARRAY_SIZE;
     }
 
     /**
@@ -391,7 +420,7 @@
     public void add(int index, E element) {
         rangeCheckForAdd(index);
 
-        ensureCapacity(size+1);  // Increments modCount!!
+        ensureCapacity(size + 1);  // Increments modCount!!
         System.arraycopy(elementData, index, elementData, index + 1,
                          size - index);
         elementData[index] = element;
--- a/jdk/src/share/classes/java/util/Hashtable.java	Fri May 07 16:11:13 2010 +0100
+++ b/jdk/src/share/classes/java/util/Hashtable.java	Sun May 09 00:59:30 2010 -0700
@@ -365,6 +365,14 @@
     }
 
     /**
+     * The maximum size of array to allocate.
+     * Some VMs reserve some header words in an array.
+     * Attempts to allocate larger arrays may result in
+     * OutOfMemoryError: Requested array size exceeds VM limit
+     */
+    private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8;
+
+    /**
      * Increases the capacity of and internally reorganizes this
      * hashtable, in order to accommodate and access its entries more
      * efficiently.  This method is called automatically when the
@@ -375,7 +383,14 @@
         int oldCapacity = table.length;
         Entry[] oldMap = table;
 
-        int newCapacity = oldCapacity * 2 + 1;
+        // overflow-conscious code
+        int newCapacity = (oldCapacity << 1) + 1;
+        if (newCapacity - MAX_ARRAY_SIZE > 0) {
+            if (oldCapacity == MAX_ARRAY_SIZE)
+                // Keep running with MAX_ARRAY_SIZE buckets
+                return;
+            newCapacity = MAX_ARRAY_SIZE;
+        }
         Entry[] newMap = new Entry[newCapacity];
 
         modCount++;
--- a/jdk/src/share/classes/java/util/PriorityQueue.java	Fri May 07 16:11:13 2010 +0100
+++ b/jdk/src/share/classes/java/util/PriorityQueue.java	Sun May 09 00:59:30 2010 -0700
@@ -236,25 +236,38 @@
     }
 
     /**
+     * The maximum size of array to allocate.
+     * Some VMs reserve some header words in an array.
+     * Attempts to allocate larger arrays may result in
+     * OutOfMemoryError: Requested array size exceeds VM limit
+     */
+    private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8;
+
+    /**
      * Increases the capacity of the array.
      *
      * @param minCapacity the desired minimum capacity
      */
     private void grow(int minCapacity) {
-        if (minCapacity < 0) // overflow
-            throw new OutOfMemoryError();
         int oldCapacity = queue.length;
         // Double size if small; else grow by 50%
-        int newCapacity = ((oldCapacity < 64)?
-                           ((oldCapacity + 1) * 2):
-                           ((oldCapacity / 2) * 3));
-        if (newCapacity < 0) // overflow
-            newCapacity = Integer.MAX_VALUE;
-        if (newCapacity < minCapacity)
-            newCapacity = minCapacity;
+        int newCapacity = oldCapacity + ((oldCapacity < 64) ?
+                                         (oldCapacity + 2) :
+                                         (oldCapacity >> 1));
+        // overflow-conscious code
+        if (newCapacity - MAX_ARRAY_SIZE > 0)
+            newCapacity = hugeCapacity(minCapacity);
         queue = Arrays.copyOf(queue, newCapacity);
     }
 
+    private static int hugeCapacity(int minCapacity) {
+        if (minCapacity < 0) // overflow
+            throw new OutOfMemoryError();
+        return (minCapacity > MAX_ARRAY_SIZE) ?
+            Integer.MAX_VALUE :
+            MAX_ARRAY_SIZE;
+    }
+
     /**
      * Inserts the specified element into this priority queue.
      *
--- a/jdk/src/share/classes/java/util/Vector.java	Fri May 07 16:11:13 2010 +0100
+++ b/jdk/src/share/classes/java/util/Vector.java	Sun May 09 00:59:30 2010 -0700
@@ -235,16 +235,37 @@
      * @see #ensureCapacity(int)
      */
     private void ensureCapacityHelper(int minCapacity) {
+        // overflow-conscious code
+        if (minCapacity - elementData.length > 0)
+            grow(minCapacity);
+    }
+
+    /**
+     * The maximum size of array to allocate.
+     * Some VMs reserve some header words in an array.
+     * Attempts to allocate larger arrays may result in
+     * OutOfMemoryError: Requested array size exceeds VM limit
+     */
+    private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8;
+
+    private void grow(int minCapacity) {
+        // overflow-conscious code
         int oldCapacity = elementData.length;
-        if (minCapacity > oldCapacity) {
-            Object[] oldData = elementData;
-            int newCapacity = (capacityIncrement > 0) ?
-                (oldCapacity + capacityIncrement) : (oldCapacity * 2);
-            if (newCapacity < minCapacity) {
-                newCapacity = minCapacity;
-            }
-            elementData = Arrays.copyOf(elementData, newCapacity);
-        }
+        int newCapacity = oldCapacity + ((capacityIncrement > 0) ?
+                                         capacityIncrement : oldCapacity);
+        if (newCapacity - minCapacity < 0)
+            newCapacity = minCapacity;
+        if (newCapacity - MAX_ARRAY_SIZE > 0)
+            newCapacity = hugeCapacity(minCapacity);
+        elementData = Arrays.copyOf(elementData, newCapacity);
+    }
+
+    private static int hugeCapacity(int minCapacity) {
+        if (minCapacity < 0) // overflow
+            throw new OutOfMemoryError();
+        return (minCapacity > MAX_ARRAY_SIZE) ?
+            Integer.MAX_VALUE :
+            MAX_ARRAY_SIZE;
     }
 
     /**