From 153b78f479502112cbae2d92ce0634e659bbc5fd Mon Sep 17 00:00:00 2001 From: Joel Dice Date: Thu, 14 Jun 2012 10:55:03 -0600 Subject: [PATCH] fix ArrayList performance issue The ArrayList(Collection) constructor was allocating two arrays instead of one due to an off-by-one error in ArrayList.grow. This commit fixes that and makes grow and shrink more robust. --- classpath/java/util/ArrayList.java | 25 +++++++++++++------------ test/List.java | 15 +++++++++++++++ 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/classpath/java/util/ArrayList.java b/classpath/java/util/ArrayList.java index d7a61d61ed..3935aa44ec 100644 --- a/classpath/java/util/ArrayList.java +++ b/classpath/java/util/ArrayList.java @@ -29,14 +29,15 @@ public class ArrayList extends AbstractList implements java.io.Serializabl addAll(source); } - private void grow() { - if (array == null || size >= array.length) { - resize(array == null ? MinimumCapacity : array.length * 2); + private void grow(int newSize) { + if (array == null || newSize > array.length) { + resize(Math.max(newSize, array == null + ? MinimumCapacity : array.length * 2)); } } - private void shrink() { - if (array.length / 2 >= MinimumCapacity && size <= array.length / 3) { + private void shrink(int newSize) { + if (array.length / 2 >= MinimumCapacity && newSize <= array.length / 3) { resize(array.length / 2); } } @@ -74,16 +75,15 @@ public class ArrayList extends AbstractList implements java.io.Serializabl } public void add(int index, T element) { - size = Math.max(size+1, index+1); - grow(); + int newSize = Math.max(size+1, index+1); + grow(newSize); + size = newSize; System.arraycopy(array, index, array, index+1, size-index-1); array[index] = element; } public boolean add(T element) { - ++ size; - grow(); - array[size - 1] = element; + add(size, element); return true; } @@ -137,8 +137,9 @@ public class ArrayList extends AbstractList implements java.io.Serializabl System.arraycopy(array, index + 1, array, index, size - index); } - -- size; - shrink(); + int newSize = size - 1; + shrink(newSize); + size = newSize; return v; } diff --git a/test/List.java b/test/List.java index 797aefbb7f..c7d73d93fc 100644 --- a/test/List.java +++ b/test/List.java @@ -131,6 +131,20 @@ public class List { expect(l.size() == 2); } + private static void testGrow() { + ArrayList foo = new ArrayList(2); + foo.add(0); + foo.add(1); + foo.add(2); // first grow + foo.add(3); + foo.add(4); // second grow + foo.add(5); + + for (int i = 0; i < foo.size(); i++) { + expect(i == foo.get(i)); + } + } + public static void main(String args[]) { ArrayList l = new ArrayList(); l.add(1); l.add(2); l.add(3); l.add(4); l.add(5); @@ -155,5 +169,6 @@ public class List { testIterators2(new ArrayList()); testIterators2(new LinkedList()); + testGrow(); } }