8362958: Unnecessary copying / sorting in Streams using Comparator.naturalOrder()

Reviewed-by: vklang, liach
This commit is contained in:
Patrick Strawderman 2025-11-25 10:13:57 +00:00 committed by Viktor Klang
parent 42f3333524
commit 67ef81eb78
4 changed files with 42 additions and 30 deletions

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2017, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -108,13 +108,10 @@ final class SortedOps {
* {@code Comparable}.
*/
OfRef(AbstractPipeline<?, T, ?> 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<? super T> comp = (Comparator<? super T>) Comparator.naturalOrder();
this.comparator = comp;
this(upstream, comp);
}
/**
@ -123,10 +120,13 @@ final class SortedOps {
* @param comparator The comparator to be used to evaluate ordering.
*/
OfRef(AbstractPipeline<?, T, ?> upstream, Comparator<? super T> comparator) {
Objects.requireNonNull(comparator);
boolean isNaturalSort = Comparator.naturalOrder().equals(comparator);
this.comparator = comparator;
this.isNaturalSort = isNaturalSort;
super(upstream, StreamShape.REFERENCE,
StreamOpFlag.IS_ORDERED | StreamOpFlag.NOT_SORTED);
this.isNaturalSort = false;
this.comparator = Objects.requireNonNull(comparator);
StreamOpFlag.IS_ORDERED |
(isNaturalSort ? StreamOpFlag.IS_SORTED : StreamOpFlag.NOT_SORTED));
}
@Override

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2017, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -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 @@ enum StreamOpFlag {
*
* @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,14 +749,15 @@ enum StreamOpFlag {
*/
static int fromCharacteristics(Spliterator<?> spliterator) {
int characteristics = spliterator.characteristics();
if ((characteristics & Spliterator.SORTED) != 0 && spliterator.getComparator() != null) {
// 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;
}
/**

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -335,20 +335,20 @@ public class StreamOpFlagsTest {
}
public void testSpliteratorSorted() {
class SortedEmptySpliterator implements Spliterator<Object> {
final Comparator<Object> c;
class SortedEmptySpliterator<T extends Comparable<? super T>> implements Spliterator<T> {
final Comparator<T> c;
SortedEmptySpliterator(Comparator<Object> c) {
SortedEmptySpliterator(Comparator<T> c) {
this.c = c;
}
@Override
public Spliterator<Object> trySplit() {
public Spliterator<T> trySplit() {
return null;
}
@Override
public boolean tryAdvance(Consumer<? super Object> action) {
public boolean tryAdvance(Consumer<? super T> action) {
return false;
}
@ -363,19 +363,24 @@ public class StreamOpFlagsTest {
}
@Override
public Comparator<? super Object> getComparator() {
public Comparator<? super T> 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<String>(Comparator.naturalOrder()));
assertEquals(flags, StreamOpFlag.IS_SORTED);
}
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -25,6 +25,7 @@ package org.openjdk.tests.java.util.stream;
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 class SortedOpTest extends OpTestCase {
Collections.reverse(to10);
assertSorted(to10.stream().sorted().iterator());
assertSorted(to10.stream().sorted(Comparator.naturalOrder()).iterator());
Spliterator<Integer> 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));
}