JDK-6307475 : (coll) Optimize AbstractCollection.toArray()
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.util:collections
  • Affected Version: 6
  • Priority: P4
  • Status: Closed
  • Resolution: Not an Issue
  • OS: generic
  • CPU: generic
  • Submitted: 2005-08-08
  • Updated: 2017-05-19
  • Resolved: 2006-05-13
Related Reports
Relates :  
JDK-6377302 - AbstractCollection.toArray is not safe for concurrent collections
Description
AbstractCollection.toArray calls hasNext() and next() in a loop,
but the hasNext() calls are unnecessary since the number of elements is known
ahead of time, since size() has already been called.

We should provide the same optimization already used in toArray(T[]).
Comments
EVALUATION We are rewriting toArray to be safe for concurrent collections, 6377302: AbstractCollection.toArray is not safe for concurrent collections so this small optimization is inappropriate.
13-05-2006
EVALUATION Hmmmm. After some consideration, the small performance improvement is probably not worth the small incompatibility. If/when we have a coherent policy on concurrent modification in AbstractCollection, we could try fixing this then. E.g. we could decide to try to return all the elements returned by the iterator, even if there are more or less than size() of them. Or we could decide to throw ConcurrentModificationException. The current behavior of returning an incompletely filled array or throwing IndexOutOfBoundsException is unreasonable, but hard to change.
07-09-2005
EVALUATION Doug Lea writes: "Probably worth doing but it has a tiny compatibility risk. If the number of elements shrink during traversal, current version would leave some array slots unfilled, but new version will blow up. (We're familiar with this issue in java.util.concurrent, where in most cases we need to implement toArray by first traversing elements (without calling size()) and placing into an ArrayList, and then returning its toArray.)" Yes... - but the other toArray method implementation has always used the same optimization - concurrently modified non-concurrent collections will get ConcurrentModificationException in most cases. - if the collection grows during traversal, old version will blow up, new version will leave some elements uncopied. Probably best to check for hasNext() and throw ConcurrentModificationException at the end.
08-08-2005
SUGGESTED FIX --- /u/martin/ws/mustang/src/share/classes/java/util/AbstractCollection.java 2004-08-27 15:54:46.733439000 -0700 +++ /u/martin/ws/toArray/src/share/classes/java/util/AbstractCollection.java 2005-08-05 17:35:28.390521000 -0700 @@ -120,10 +120,11 @@ * @return an array containing all of the elements in this collection. */ public Object[] toArray() { - Object[] result = new Object[size()]; - Iterator<E> e = iterator(); - for (int i=0; e.hasNext(); i++) - result[i] = e.next(); + int size = size(); + Object[] result = new Object[size]; + Iterator<E> it = iterator(); + for (int i=0; i < size; i++) + result[i] = it.next(); return result; }
08-08-2005
EVALUATION Yes
08-08-2005