RFR: 8362958: Unnecessary copying / sorting in Streams using Comparator.naturalOrder() [v2]
Rémi Forax
forax at openjdk.org
Sat Nov 15 09:31:14 UTC 2025
On Tue, 11 Nov 2025 17:46:11 GMT, Rémi Forax <forax at openjdk.org> wrote:
>>> I wonder if it's not better to replace Comparator.naturalOrder() by null in the constructor of TreeSet, given that TreeSet does not provide a getter for it so the only way to get the comparator is using treeSet.spliterator().getComparator().
>>
>> That wouldn't help with the `stream().sorted(Comparator.naturalOrder())` case; the example I supplied is somewhat contrived, I originally ran into this in Guava (see the [issue](https://github.com/google/guava/issues/6187) I opened there); the library is null-hostile and so always supplies a comparator to its sorted collections. Note that this won't directly fix the issue there until they move away from their custom natural order comparator to the one from the JDK.
>
>> That wouldn't help with the `stream().sorted(Comparator.naturalOrder())` case; the example I supplied is somewhat contrived, I originally ran into this in Guava (see the [issue](https://github.com/google/guava/issues/6187) I opened there); the library is null-hostile and so always supplies a comparator to its sorted collections.
>
> If you switch from Comparator.naturalOrder()) to null, you have to do it in stream.sorted() too (and also List.sort(Comparator), Collections.sort(Comparator), Array.sort(Comparator), Collections.reverseOrder(Comparator) etc)
>
>> Note that this won't directly fix the issue there until they move away from their custom natural order comparator to the one from the JDK.
>
> yes, comparing comparators with == is brittle anyway, but at least you can make it consistent for the JDK.
> @forax
>
> > I wonder if it's not better to replace Comparator.naturalOrder() by null in the constructor of TreeSet, given that TreeSet does not provide a getter for it so the only way to get the comparator is using treeSet.spliterator().getComparator().
>
> TreeSet does have a "getter" named `comparator()`. It's defined by SortedSet. Not sure this affects where the discussion is at the moment though.
ah, thanks Stuart,
so changing the Comparator at use site is definitively not a good idea :(
-------------
PR Comment: https://git.openjdk.org/jdk/pull/28226#issuecomment-3536249353
More information about the core-libs-dev
mailing list