The implementation looks good. I would move construction of the listIterator to before the toArray() for detection of concurrent modification (growing of the list).
I believe we should fix this for 8 if possible. Mike On Jan 10 2014, at 03:21 , Paul Sandoz <paul.san...@oracle.com> wrote: > Hi, > > When we added the List.sort method the default implementation deferred to > Collections.sort. > > This is the wrong way around, since any existing use of Collections.sort say > with ArrayList will not avail of the optimal implementation provided by > ArrayList.sort. > > To resolve this the implementation of Collections.sort can be moved to > List.sort and Collections.sort can defer to List.sort. > > Code changes are here: > > http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8030848-Collections-sort/webrev/ > > I made some tweaks to Arrays.sort to preserve cases when the Comparator is > null. > > Existing tests provide good coverage and there are no regressions when i run > j.u. tests locally. > > I am not happy with the current documentation though, i think that also > requires some refactoring, moving stuff from Collections.sort to List.sort > and explicitly stating what the current implementation of Collections.sort > does. I believe this requires no spec changes even though it may appear so. > Thoughts? > > Also, i am concerned that this change could cause stack overflows for list > implementations that override sort and still defer to Collections.sort. > Implying we should fix this for 8 or quickly in an 8u. > > Paul.
