From c4fb5d0f9b3bbfb7d9f5e6b9b8e1f22c07e69bc1 Mon Sep 17 00:00:00 2001 From: Patrick Strawderman Date: Mon, 10 Nov 2025 18:11:35 -0800 Subject: [PATCH 1/8] 8362958: Unnecessary copying / sorting in Streams using Comparator.naturalOrder() --- .../share/classes/java/util/stream/SortedOps.java | 3 ++- .../share/classes/java/util/stream/StreamOpFlag.java | 11 +++++++---- .../java.base/java/util/stream/StreamOpFlagsTest.java | 5 +++++ .../openjdk/tests/java/util/stream/SortedOpTest.java | 5 +++++ 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/java.base/share/classes/java/util/stream/SortedOps.java b/src/java.base/share/classes/java/util/stream/SortedOps.java index 91892eba44fc8..9f4444a829991 100644 --- a/src/java.base/share/classes/java/util/stream/SortedOps.java +++ b/src/java.base/share/classes/java/util/stream/SortedOps.java @@ -60,7 +60,8 @@ static Stream makeRef(AbstractPipeline upstream) { */ static Stream makeRef(AbstractPipeline upstream, Comparator comparator) { - return new OfRef<>(upstream, comparator); + return Comparator.naturalOrder().equals(comparator) ? + new OfRef<>(upstream) : new OfRef<>(upstream, comparator); } /** diff --git a/src/java.base/share/classes/java/util/stream/StreamOpFlag.java b/src/java.base/share/classes/java/util/stream/StreamOpFlag.java index df98125cbc02f..78506ada61d8e 100644 --- a/src/java.base/share/classes/java/util/stream/StreamOpFlag.java +++ b/src/java.base/share/classes/java/util/stream/StreamOpFlag.java @@ -24,6 +24,7 @@ */ package java.util.stream; +import java.util.Comparator; import java.util.EnumMap; import java.util.Map; import java.util.Spliterator; @@ -738,9 +739,9 @@ static int toCharacteristics(int streamFlags) { * * @implSpec * If the spliterator is naturally {@code SORTED} (the associated - * {@code Comparator} is {@code null}) then the characteristic is converted - * to the {@link #SORTED} flag, otherwise the characteristic is not - * converted. + * {@code Comparator} is {@code null} or {@code Comparator.naturalOrder()}) then + * the characteristic is converted to the {@link #SORTED} flag, otherwise + * the characteristic is not converted. * * @param spliterator the spliterator from which to obtain characteristic * bit set. @@ -748,7 +749,9 @@ static int toCharacteristics(int streamFlags) { */ static int fromCharacteristics(Spliterator spliterator) { int characteristics = spliterator.characteristics(); - if ((characteristics & Spliterator.SORTED) != 0 && spliterator.getComparator() != null) { + if ((characteristics & Spliterator.SORTED) != 0 && + !(spliterator.getComparator() == null || + spliterator.getComparator().equals(Comparator.naturalOrder()))) { // Do not propagate the SORTED characteristic if it does not correspond // to a natural sort order return characteristics & SPLITERATOR_CHARACTERISTICS_MASK & ~Spliterator.SORTED; diff --git a/test/jdk/java/util/stream/boottest/java.base/java/util/stream/StreamOpFlagsTest.java b/test/jdk/java/util/stream/boottest/java.base/java/util/stream/StreamOpFlagsTest.java index aa47aa2a1039a..64b3841c57819 100644 --- a/test/jdk/java/util/stream/boottest/java.base/java/util/stream/StreamOpFlagsTest.java +++ b/test/jdk/java/util/stream/boottest/java.base/java/util/stream/StreamOpFlagsTest.java @@ -377,5 +377,10 @@ public Comparator getComparator() { int flags = StreamOpFlag.fromCharacteristics(new SortedEmptySpliterator((a, b) -> 0)); assertEquals(flags, 0); } + + { + int flags = StreamOpFlag.fromCharacteristics(new SortedEmptySpliterator(Comparator.naturalOrder())); + assertEquals(flags, StreamOpFlag.IS_SORTED); + } } } diff --git a/test/jdk/java/util/stream/test/org/openjdk/tests/java/util/stream/SortedOpTest.java b/test/jdk/java/util/stream/test/org/openjdk/tests/java/util/stream/SortedOpTest.java index 3ca690e9096d3..d8c535a6e8f61 100644 --- a/test/jdk/java/util/stream/test/org/openjdk/tests/java/util/stream/SortedOpTest.java +++ b/test/jdk/java/util/stream/test/org/openjdk/tests/java/util/stream/SortedOpTest.java @@ -25,6 +25,7 @@ import org.testng.annotations.Test; import java.util.*; +import java.util.Comparator; import java.util.Spliterators; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.BiFunction; @@ -115,10 +116,14 @@ public void testSorted() { Collections.reverse(to10); assertSorted(to10.stream().sorted().iterator()); + assertSorted(to10.stream().sorted(Comparator.naturalOrder()).iterator()); Spliterator s = to10.stream().sorted().spliterator(); assertTrue(s.hasCharacteristics(Spliterator.SORTED)); + s = to10.stream().sorted(Comparator.naturalOrder()).spliterator(); + assertTrue(s.hasCharacteristics(Spliterator.SORTED)); + s = to10.stream().sorted(cInteger.reversed()).spliterator(); assertFalse(s.hasCharacteristics(Spliterator.SORTED)); } From 7c8fea3a319a296e63ce6d28c42f3d84c8d39778 Mon Sep 17 00:00:00 2001 From: Patrick Strawderman Date: Tue, 11 Nov 2025 08:31:27 -0800 Subject: [PATCH 2/8] Update src/java.base/share/classes/java/util/stream/StreamOpFlag.java Simplify comparison Co-authored-by: Viktor Klang --- .../share/classes/java/util/stream/StreamOpFlag.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/java.base/share/classes/java/util/stream/StreamOpFlag.java b/src/java.base/share/classes/java/util/stream/StreamOpFlag.java index 78506ada61d8e..392bedb51d472 100644 --- a/src/java.base/share/classes/java/util/stream/StreamOpFlag.java +++ b/src/java.base/share/classes/java/util/stream/StreamOpFlag.java @@ -751,7 +751,9 @@ static int fromCharacteristics(Spliterator spliterator) { int characteristics = spliterator.characteristics(); if ((characteristics & Spliterator.SORTED) != 0 && !(spliterator.getComparator() == null || - spliterator.getComparator().equals(Comparator.naturalOrder()))) { + if ((characteristics & Spliterator.SORTED) != 0 && + spliterator.getComparator() instanceof Comparator c && + !c.equals(Comparator.naturalOrder()))) { // Do not propagate the SORTED characteristic if it does not correspond // to a natural sort order return characteristics & SPLITERATOR_CHARACTERISTICS_MASK & ~Spliterator.SORTED; From caeb8f379fbf40f514ee4adfedae7dc88546c979 Mon Sep 17 00:00:00 2001 From: Patrick Strawderman Date: Tue, 11 Nov 2025 09:07:23 -0800 Subject: [PATCH 3/8] Check for Comparator.naturalOrder() in OfRef constructor --- .../classes/java/util/stream/SortedOps.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/java.base/share/classes/java/util/stream/SortedOps.java b/src/java.base/share/classes/java/util/stream/SortedOps.java index 9f4444a829991..627d2f0b5f03d 100644 --- a/src/java.base/share/classes/java/util/stream/SortedOps.java +++ b/src/java.base/share/classes/java/util/stream/SortedOps.java @@ -60,8 +60,7 @@ static Stream makeRef(AbstractPipeline upstream) { */ static Stream makeRef(AbstractPipeline upstream, Comparator comparator) { - return Comparator.naturalOrder().equals(comparator) ? - new OfRef<>(upstream) : new OfRef<>(upstream, comparator); + return new OfRef<>(upstream, comparator); } /** @@ -109,13 +108,10 @@ private static final class OfRef extends ReferencePipeline.StatefulOp { * {@code Comparable}. */ OfRef(AbstractPipeline upstream) { - super(upstream, StreamShape.REFERENCE, - StreamOpFlag.IS_ORDERED | StreamOpFlag.IS_SORTED); - this.isNaturalSort = true; // Will throw CCE when we try to sort if T is not Comparable @SuppressWarnings("unchecked") Comparator comp = (Comparator) Comparator.naturalOrder(); - this.comparator = comp; + this(upstream, comp); } /** @@ -124,10 +120,13 @@ private static final class OfRef extends ReferencePipeline.StatefulOp { * @param comparator The comparator to be used to evaluate ordering. */ OfRef(AbstractPipeline upstream, Comparator comparator) { + Objects.requireNonNull(comparator); + boolean isNaturalSort = Comparator.naturalOrder().equals(comparator); super(upstream, StreamShape.REFERENCE, - StreamOpFlag.IS_ORDERED | StreamOpFlag.NOT_SORTED); - this.isNaturalSort = false; - this.comparator = Objects.requireNonNull(comparator); + StreamOpFlag.IS_ORDERED | + (isNaturalSort ? StreamOpFlag.SORTED : StreamOpFlag.NOT_SORTED)); + this.isNaturalSort = isNaturalSort; + this.comparator = comparator; } @Override From 4c48f937fb29688fdb46e7b752581a0e8f56983a Mon Sep 17 00:00:00 2001 From: Patrick Strawderman Date: Tue, 11 Nov 2025 09:50:50 -0800 Subject: [PATCH 4/8] Fix bad merge --- .../share/classes/java/util/stream/StreamOpFlag.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/java.base/share/classes/java/util/stream/StreamOpFlag.java b/src/java.base/share/classes/java/util/stream/StreamOpFlag.java index 392bedb51d472..0cd90c6a5c9ca 100644 --- a/src/java.base/share/classes/java/util/stream/StreamOpFlag.java +++ b/src/java.base/share/classes/java/util/stream/StreamOpFlag.java @@ -750,10 +750,8 @@ static int toCharacteristics(int streamFlags) { static int fromCharacteristics(Spliterator spliterator) { int characteristics = spliterator.characteristics(); if ((characteristics & Spliterator.SORTED) != 0 && - !(spliterator.getComparator() == null || - if ((characteristics & Spliterator.SORTED) != 0 && - spliterator.getComparator() instanceof Comparator c && - !c.equals(Comparator.naturalOrder()))) { + (spliterator.getComparator() instanceof Comparator c && + !c.equals(Comparator.naturalOrder()))) { // Do not propagate the SORTED characteristic if it does not correspond // to a natural sort order return characteristics & SPLITERATOR_CHARACTERISTICS_MASK & ~Spliterator.SORTED; From ea73382c79ccec9b9b0b54720cdf58ed9b88765e Mon Sep 17 00:00:00 2001 From: Patrick Strawderman Date: Tue, 11 Nov 2025 10:28:16 -0800 Subject: [PATCH 5/8] Fix SortOps.OfRef logic --- src/java.base/share/classes/java/util/stream/SortedOps.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java.base/share/classes/java/util/stream/SortedOps.java b/src/java.base/share/classes/java/util/stream/SortedOps.java index 627d2f0b5f03d..9b043def821e0 100644 --- a/src/java.base/share/classes/java/util/stream/SortedOps.java +++ b/src/java.base/share/classes/java/util/stream/SortedOps.java @@ -124,7 +124,7 @@ private static final class OfRef extends ReferencePipeline.StatefulOp { boolean isNaturalSort = Comparator.naturalOrder().equals(comparator); super(upstream, StreamShape.REFERENCE, StreamOpFlag.IS_ORDERED | - (isNaturalSort ? StreamOpFlag.SORTED : StreamOpFlag.NOT_SORTED)); + (isNaturalSort ? StreamOpFlag.IS_SORTED : StreamOpFlag.NOT_SORTED)); this.isNaturalSort = isNaturalSort; this.comparator = comparator; } From c5bf7d569f117a767291b92d5d0c48abae7f93d3 Mon Sep 17 00:00:00 2001 From: Patrick Strawderman Date: Tue, 11 Nov 2025 12:55:28 -0800 Subject: [PATCH 6/8] Clean up --- .../classes/java/util/stream/StreamOpFlag.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/java.base/share/classes/java/util/stream/StreamOpFlag.java b/src/java.base/share/classes/java/util/stream/StreamOpFlag.java index 0cd90c6a5c9ca..2635e2163c1c0 100644 --- a/src/java.base/share/classes/java/util/stream/StreamOpFlag.java +++ b/src/java.base/share/classes/java/util/stream/StreamOpFlag.java @@ -749,16 +749,15 @@ static int toCharacteristics(int streamFlags) { */ static int fromCharacteristics(Spliterator spliterator) { int characteristics = spliterator.characteristics(); - if ((characteristics & Spliterator.SORTED) != 0 && - (spliterator.getComparator() instanceof Comparator c && - !c.equals(Comparator.naturalOrder()))) { - // Do not propagate the SORTED characteristic if it does not correspond - // to a natural sort order - return characteristics & SPLITERATOR_CHARACTERISTICS_MASK & ~Spliterator.SORTED; - } - else { - return characteristics & SPLITERATOR_CHARACTERISTICS_MASK; + if ((characteristics & Spliterator.SORTED) != 0) { + Comparator comparator = spliterator.getComparator(); + if (comparator != null && !Comparator.naturalOrder().equals(comparator)) { + // Do not propagate the SORTED characteristic if it does not correspond + // to a natural sort order + return characteristics & SPLITERATOR_CHARACTERISTICS_MASK & ~Spliterator.SORTED; + } } + return characteristics & SPLITERATOR_CHARACTERISTICS_MASK; } /** From 3f339ad97ba2e2d7d924b550e266815272829356 Mon Sep 17 00:00:00 2001 From: Patrick Strawderman Date: Wed, 12 Nov 2025 18:46:11 -0800 Subject: [PATCH 7/8] Assign fields before super call --- src/java.base/share/classes/java/util/stream/SortedOps.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/java.base/share/classes/java/util/stream/SortedOps.java b/src/java.base/share/classes/java/util/stream/SortedOps.java index 9b043def821e0..e09b8b9f96cfb 100644 --- a/src/java.base/share/classes/java/util/stream/SortedOps.java +++ b/src/java.base/share/classes/java/util/stream/SortedOps.java @@ -122,11 +122,11 @@ private static final class OfRef extends ReferencePipeline.StatefulOp { OfRef(AbstractPipeline upstream, Comparator comparator) { Objects.requireNonNull(comparator); boolean isNaturalSort = Comparator.naturalOrder().equals(comparator); + this.comparator = comparator; + this.isNaturalSort = isNaturalSort; super(upstream, StreamShape.REFERENCE, StreamOpFlag.IS_ORDERED | (isNaturalSort ? StreamOpFlag.IS_SORTED : StreamOpFlag.NOT_SORTED)); - this.isNaturalSort = isNaturalSort; - this.comparator = comparator; } @Override From 9452d7474d90557faad63ad0a76043e3d0f6e0fa Mon Sep 17 00:00:00 2001 From: Patrick Strawderman Date: Wed, 12 Nov 2025 22:38:34 -0800 Subject: [PATCH 8/8] Fix StreamOpFlagsTest --- .../java/util/stream/StreamOpFlagsTest.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/jdk/java/util/stream/boottest/java.base/java/util/stream/StreamOpFlagsTest.java b/test/jdk/java/util/stream/boottest/java.base/java/util/stream/StreamOpFlagsTest.java index 64b3841c57819..9c0ece14c8b95 100644 --- a/test/jdk/java/util/stream/boottest/java.base/java/util/stream/StreamOpFlagsTest.java +++ b/test/jdk/java/util/stream/boottest/java.base/java/util/stream/StreamOpFlagsTest.java @@ -335,20 +335,20 @@ private void assertSpliteratorCharacteristicsMask(int actual, int expected) { } public void testSpliteratorSorted() { - class SortedEmptySpliterator implements Spliterator { - final Comparator c; + class SortedEmptySpliterator> implements Spliterator { + final Comparator c; - SortedEmptySpliterator(Comparator c) { + SortedEmptySpliterator(Comparator c) { this.c = c; } @Override - public Spliterator trySplit() { + public Spliterator trySplit() { return null; } @Override - public boolean tryAdvance(Consumer action) { + public boolean tryAdvance(Consumer action) { return false; } @@ -363,23 +363,23 @@ public int characteristics() { } @Override - public Comparator getComparator() { + public Comparator getComparator() { return c; } - }; + } { - int flags = StreamOpFlag.fromCharacteristics(new SortedEmptySpliterator(null)); + int flags = StreamOpFlag.fromCharacteristics(new SortedEmptySpliterator<>(null)); assertEquals(flags, StreamOpFlag.IS_SORTED); } { - int flags = StreamOpFlag.fromCharacteristics(new SortedEmptySpliterator((a, b) -> 0)); + int flags = StreamOpFlag.fromCharacteristics(new SortedEmptySpliterator<>((a, b) -> 0)); assertEquals(flags, 0); } { - int flags = StreamOpFlag.fromCharacteristics(new SortedEmptySpliterator(Comparator.naturalOrder())); + int flags = StreamOpFlag.fromCharacteristics(new SortedEmptySpliterator(Comparator.naturalOrder())); assertEquals(flags, StreamOpFlag.IS_SORTED); } }