RFR 8030848: Collections.sort(List l, Comparator) should defer to List.sort(Comparator )

Mike Duigou mike.duigou at oracle.com
Fri Jan 10 17:56:38 UTC 2014


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.sandoz at 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.




More information about the core-libs-dev mailing list