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