From 12b4cb1f85bc9723c90716d9b279eaafae750788 Mon Sep 17 00:00:00 2001 From: Agyei Holy Date: Fri, 24 Apr 2026 04:37:34 -0500 Subject: [PATCH] Return null from Spliterator.getComparator() for natural ordering. Per the Spliterator.getComparator() contract, implementations whose source is sorted in natural order must return null. The JDK uses this signal to clear the SORTED characteristic when the comparator is non-null, which in turn disables Stream.sorted()'s short-circuit optimization (SortedOps line 136-139), causing unnecessary re-sorting of already-sorted data. ImmutableSortedSet.spliterator() and CollectSpliterators.indexed() (used by ImmutableSortedAsList.spliterator()) returned Ordering.natural() instead of null for naturally-ordered sources, so code like ImmutableSortedSet.of(1, 2, 3, 4).stream().sorted().collect(toList()); incurred a full re-sort rather than a no-op. This change normalizes the two well-known natural-ordering singletons (Ordering.natural() and Comparator.naturalOrder()) to null at the getComparator() boundary via a shared CollectSpliterators.isNaturalOrder() helper. Custom and reverse-order comparators are preserved unchanged. Fixes #6187. Made-with: Cursor --- .../collect/CollectSpliteratorsTest.java | 46 +++++++++++++++++++ .../collect/ImmutableSortedSetTest.java | 38 +++++++++++++++ .../common/collect/CollectSpliterators.java | 19 +++++++- .../common/collect/ImmutableSortedSet.java | 7 ++- 4 files changed, 107 insertions(+), 3 deletions(-) diff --git a/guava-tests/test/com/google/common/collect/CollectSpliteratorsTest.java b/guava-tests/test/com/google/common/collect/CollectSpliteratorsTest.java index 7e49123e11bc..a2cbb2c4290e 100644 --- a/guava-tests/test/com/google/common/collect/CollectSpliteratorsTest.java +++ b/guava-tests/test/com/google/common/collect/CollectSpliteratorsTest.java @@ -23,6 +23,7 @@ import com.google.common.collect.testing.SpliteratorTester; import java.util.ArrayList; import java.util.Arrays; +import java.util.Comparator; import java.util.List; import java.util.Spliterator; import java.util.stream.DoubleStream; @@ -111,4 +112,49 @@ public void testMultisetsSpliterator() { multiset.spliterator().forEachRemaining(actualValues::add); assertThat(multiset).containsExactly("a", "a", "a", "b", "c", "c").inOrder(); } + + // Regression tests for https://github.com/google/guava/issues/6187. + + public void testIsNaturalOrder_null() { + assertThat(CollectSpliterators.isNaturalOrder(null)).isTrue(); + } + + public void testIsNaturalOrder_orderingNatural() { + assertThat(CollectSpliterators.isNaturalOrder(Ordering.natural())).isTrue(); + } + + public void testIsNaturalOrder_comparatorNaturalOrder() { + assertThat(CollectSpliterators.isNaturalOrder(Comparator.naturalOrder())).isTrue(); + } + + public void testIsNaturalOrder_reverseOrder_false() { + assertThat(CollectSpliterators.isNaturalOrder(Comparator.reverseOrder())).isFalse(); + assertThat(CollectSpliterators.isNaturalOrder(Ordering.natural().reverse())).isFalse(); + } + + public void testIsNaturalOrder_customComparator_false() { + Comparator custom = (a, b) -> a - b; + assertThat(CollectSpliterators.isNaturalOrder(custom)).isFalse(); + } + + public void testIndexedSortedSpliterator_naturalOrderReturnsNull() { + Spliterator spliterator = + CollectSpliterators.indexed( + 3, Spliterator.SORTED, i -> i + 1, Ordering.natural()); + assertThat(spliterator.getComparator()).isNull(); + assertThat(spliterator.hasCharacteristics(Spliterator.SORTED)).isTrue(); + } + + public void testIndexedSortedSpliterator_comparatorNaturalOrderReturnsNull() { + Spliterator spliterator = + CollectSpliterators.indexed(3, Spliterator.SORTED, i -> i + 1, Comparator.naturalOrder()); + assertThat(spliterator.getComparator()).isNull(); + } + + public void testIndexedSortedSpliterator_customComparatorIsPreserved() { + Comparator reverse = Comparator.reverseOrder(); + Spliterator spliterator = + CollectSpliterators.indexed(3, Spliterator.SORTED, i -> 3 - i, reverse); + assertThat(spliterator.getComparator()).isEqualTo(reverse); + } } diff --git a/guava-tests/test/com/google/common/collect/ImmutableSortedSetTest.java b/guava-tests/test/com/google/common/collect/ImmutableSortedSetTest.java index 29770b36ee03..35efdb82136a 100644 --- a/guava-tests/test/com/google/common/collect/ImmutableSortedSetTest.java +++ b/guava-tests/test/com/google/common/collect/ImmutableSortedSetTest.java @@ -57,6 +57,7 @@ import java.util.NoSuchElementException; import java.util.Set; import java.util.SortedSet; +import java.util.Spliterator; import java.util.TreeSet; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.BiPredicate; @@ -1221,6 +1222,43 @@ public void testBuilderAsymptotics() { // hopefully something quadratic would have more digits } + // Regression tests for https://github.com/google/guava/issues/6187: + // Spliterator.getComparator() must return null for naturally-ordered sources so that + // stream optimizations (e.g. Stream.sorted() as a no-op) kick in. + + public void testSpliteratorGetComparator_naturalOrdering_returnsNull() { + ImmutableSortedSet set = ImmutableSortedSet.of(1, 2, 3, 4); + assertThat(set.spliterator().getComparator()).isNull(); + } + + public void testSpliteratorGetComparator_comparatorNaturalOrder_returnsNull() { + ImmutableSortedSet set = + ImmutableSortedSet.copyOf(Comparator.naturalOrder(), asList(1, 2, 3, 4)); + assertThat(set.spliterator().getComparator()).isNull(); + } + + public void testSpliteratorGetComparator_customOrdering_returnsComparator() { + Comparator reverse = Comparator.reverseOrder(); + ImmutableSortedSet set = ImmutableSortedSet.copyOf(reverse, asList(1, 2, 3, 4)); + assertThat(set.spliterator().getComparator()).isEqualTo(reverse); + } + + public void testSpliteratorHasSortedCharacteristic() { + ImmutableSortedSet set = ImmutableSortedSet.of(1, 2, 3, 4); + assertThat(set.spliterator().hasCharacteristics(Spliterator.SORTED)).isTrue(); + } + + public void testAsListSpliteratorGetComparator_naturalOrdering_returnsNull() { + ImmutableSortedSet set = ImmutableSortedSet.of(1, 2, 3, 4); + assertThat(set.asList().spliterator().getComparator()).isNull(); + } + + public void testAsListSpliteratorGetComparator_customOrdering_returnsComparator() { + Comparator reverse = Comparator.reverseOrder(); + ImmutableSortedSet set = ImmutableSortedSet.copyOf(reverse, asList(1, 2, 3, 4)); + assertThat(set.asList().spliterator().getComparator()).isEqualTo(reverse); + } + private static SortedSet newTreeSetWithComparator( Comparator comparator, E... elements) { SortedSet set = new TreeSet<>(comparator); diff --git a/guava/src/com/google/common/collect/CollectSpliterators.java b/guava/src/com/google/common/collect/CollectSpliterators.java index 612f19bcffeb..895c90b20ec5 100644 --- a/guava/src/com/google/common/collect/CollectSpliterators.java +++ b/guava/src/com/google/common/collect/CollectSpliterators.java @@ -40,6 +40,20 @@ final class CollectSpliterators { private CollectSpliterators() {} + /** + * Returns whether {@code comparator} represents the natural ordering of its elements. + * + *

Recognizes the two well-known natural-ordering singletons: {@link Ordering#natural()} and + * {@link Comparator#naturalOrder()}. Any comparator that compares equal to one of these (via + * {@link Object#equals}) is treated as natural order so that {@link Spliterator#getComparator()} + * can return {@code null} as required by its contract. + */ + static boolean isNaturalOrder(@Nullable Comparator comparator) { + return comparator == null + || Ordering.natural().equals(comparator) + || Comparator.naturalOrder().equals(comparator); + } + static Spliterator indexed( int size, int extraCharacteristics, IntFunction function) { return indexed(size, extraCharacteristics, function, null); @@ -92,7 +106,10 @@ public int characteristics() { @Override public @Nullable Comparator getComparator() { if (hasCharacteristics(Spliterator.SORTED)) { - return comparator; + // Per Spliterator.getComparator()'s contract, null signals that the source is + // sorted in natural order. Normalizing natural-ordering singletons to null allows + // Stream.sorted() to short-circuit instead of re-sorting already-sorted data. + return isNaturalOrder(comparator) ? null : comparator; } else { throw new IllegalStateException(); } diff --git a/guava/src/com/google/common/collect/ImmutableSortedSet.java b/guava/src/com/google/common/collect/ImmutableSortedSet.java index d072644543eb..824a14823517 100644 --- a/guava/src/com/google/common/collect/ImmutableSortedSet.java +++ b/guava/src/com/google/common/collect/ImmutableSortedSet.java @@ -832,8 +832,11 @@ public boolean tryAdvance(Consumer action) { } @Override - public Comparator getComparator() { - return comparator; + public @Nullable Comparator getComparator() { + // Per Spliterator.getComparator()'s contract, null signals that the source is sorted in + // natural order. Normalizing natural-ordering singletons to null allows Stream.sorted() + // to short-circuit instead of re-sorting already-sorted data. + return CollectSpliterators.isNaturalOrder(comparator) ? null : comparator; } }; }