Skip to content

Commit 843e726

Browse files
collect: Return null from Spliterator.getComparator() for natural ordering
Per the Spliterator.getComparator() contract, return null to indicate natural ordering. This enables stream optimizations where sorted() becomes a no-op for already-sorted collections using natural ordering. Previously, ImmutableSortedSet and CollectSpliterators returned Ordering.natural() instead of null, which caused the JDK stream implementation to clear the SORTED flag and re-sort unnecessarily. The fix checks for both Ordering.natural() and Comparator.naturalOrder() using reference equality (both are singletons). Fixes #6187
1 parent 50f087f commit 843e726

3 files changed

Lines changed: 39 additions & 1 deletion

File tree

guava-tests/test/com/google/common/collect/ImmutableSortedSetTest.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
import java.util.NoSuchElementException;
5656
import java.util.Set;
5757
import java.util.SortedSet;
58+
import java.util.Spliterator;
5859
import java.util.TreeSet;
5960
import java.util.function.BiPredicate;
6061
import java.util.stream.Collector;
@@ -1225,4 +1226,33 @@ public void testBuilderAsymptotics() {
12251226
assertThat(compares[0]).isAtMost(10000);
12261227
// hopefully something quadratic would have more digits
12271228
}
1229+
1230+
@GwtIncompatible // Spliterator
1231+
public void testSpliteratorComparator_naturalOrdering() {
1232+
// Per Spliterator.getComparator() contract, null indicates natural ordering
1233+
ImmutableSortedSet<Integer> set = ImmutableSortedSet.of(1, 2, 3);
1234+
Spliterator<Integer> spliterator = set.spliterator();
1235+
assertThat(spliterator.hasCharacteristics(Spliterator.SORTED)).isTrue();
1236+
assertThat(spliterator.getComparator()).isNull();
1237+
}
1238+
1239+
@GwtIncompatible // Spliterator
1240+
public void testSpliteratorComparator_customComparator() {
1241+
// Custom comparator should be returned from getComparator()
1242+
Comparator<Integer> comparator = Comparator.reverseOrder();
1243+
ImmutableSortedSet<Integer> set =
1244+
ImmutableSortedSet.orderedBy(comparator).add(1, 2, 3).build();
1245+
Spliterator<Integer> spliterator = set.spliterator();
1246+
assertThat(spliterator.hasCharacteristics(Spliterator.SORTED)).isTrue();
1247+
assertThat(spliterator.getComparator()).isEqualTo(comparator);
1248+
}
1249+
1250+
@GwtIncompatible // Spliterator
1251+
public void testAsListSpliteratorComparator_naturalOrdering() {
1252+
// asList().spliterator() should also return null for natural ordering
1253+
ImmutableSortedSet<Integer> set = ImmutableSortedSet.of(1, 2, 3);
1254+
Spliterator<Integer> spliterator = set.asList().spliterator();
1255+
assertThat(spliterator.hasCharacteristics(Spliterator.SORTED)).isTrue();
1256+
assertThat(spliterator.getComparator()).isNull();
1257+
}
12281258
}

guava/src/com/google/common/collect/CollectSpliterators.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ private CollectSpliterators() {}
5353
if (comparator != null) {
5454
checkArgument((extraCharacteristics & Spliterator.SORTED) != 0);
5555
}
56+
// Per Spliterator.getComparator() contract, return null for natural ordering
57+
@Nullable Comparator<? super T> spliteratorComparator =
58+
(comparator == Ordering.natural() || comparator == Comparator.naturalOrder())
59+
? null
60+
: comparator;
5661
final class WithCharacteristics implements Spliterator<T> {
5762
private final Spliterator.OfInt delegate;
5863

@@ -92,7 +97,7 @@ public int characteristics() {
9297
@Override
9398
public @Nullable Comparator<? super T> getComparator() {
9499
if (hasCharacteristics(Spliterator.SORTED)) {
95-
return comparator;
100+
return spliteratorComparator;
96101
} else {
97102
throw new IllegalStateException();
98103
}

guava/src/com/google/common/collect/ImmutableSortedSet.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -833,6 +833,9 @@ public boolean tryAdvance(Consumer<? super E> action) {
833833

834834
@Override
835835
public Comparator<? super E> getComparator() {
836+
if (comparator == Ordering.natural() || comparator == Comparator.naturalOrder()) {
837+
return null;
838+
}
836839
return comparator;
837840
}
838841
};

0 commit comments

Comments
 (0)