JDK 10 RFR JDK-8174267: Stream.findFirst unnecessarily always allocates an Op

[Paul Sandoz]

> On 28 Apr 2017, at 12:34, Ron Pressler <ron.pressler at oracle.com> wrote:
> 
> Hi.
> Please review the following patch, which creates static final instances of FindOp.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8174267
> 

+1


> I believe an additional test is not required as this is an implementation detail.
> 

Our existing stream tests cover this.

Paul.

> -------
> diff -r 83d37efcd2a5 src/java.base/share/classes/java/util/stream/FindOps.java
> --- a/src/java.base/share/classes/java/util/stream/FindOps.java Thu Apr 13 20:35:17 2017 +0000
> +++ b/src/java.base/share/classes/java/util/stream/FindOps.java Fri Apr 28 19:28:36 2017 +0000
> @@ -54,9 +54,10 @@
>      *        first element in the encounter order
>      * @return a {@code TerminalOp} implementing the find operation
>      */
> +    @SuppressWarnings("unchecked")
>     public static <T> TerminalOp<T, Optional<T>> makeRef(boolean mustFindFirst) {
> -        return new FindOp<>(mustFindFirst, StreamShape.REFERENCE, Optional.empty(),
> -                            Optional::isPresent, FindSink.OfRef::new);
> +        return (TerminalOp<T, Optional<T>>)
> +                (mustFindFirst ? FindSink.OfRef.OP_FIND_FIRST : FindSink.OfRef.OP_FIND_ANY);
>     }
> 
>     /**
> @@ -67,8 +68,7 @@
>      * @return a {@code TerminalOp} implementing the find operation
>      */
>     public static TerminalOp<Integer, OptionalInt> makeInt(boolean mustFindFirst) {
> -        return new FindOp<>(mustFindFirst, StreamShape.INT_VALUE, OptionalInt.empty(),
> -                            OptionalInt::isPresent, FindSink.OfInt::new);
> +        return mustFindFirst ? FindSink.OfInt.OP_FIND_FIRST : FindSink.OfInt.OP_FIND_ANY;
>     }
> 
>     /**
> @@ -79,8 +79,7 @@
>      * @return a {@code TerminalOp} implementing the find operation
>      */
>     public static TerminalOp<Long, OptionalLong> makeLong(boolean mustFindFirst) {
> -        return new FindOp<>(mustFindFirst, StreamShape.LONG_VALUE, OptionalLong.empty(),
> -                            OptionalLong::isPresent, FindSink.OfLong::new);
> +        return mustFindFirst ? FindSink.OfLong.OP_FIND_FIRST : FindSink.OfLong.OP_FIND_ANY;
>     }
> 
>     /**
> @@ -91,8 +90,7 @@
>      * @return a {@code TerminalOp} implementing the find operation
>      */
>     public static TerminalOp<Double, OptionalDouble> makeDouble(boolean mustFindFirst) {
> -        return new FindOp<>(mustFindFirst, StreamShape.DOUBLE_VALUE, OptionalDouble.empty(),
> -                            OptionalDouble::isPresent, FindSink.OfDouble::new);
> +        return mustFindFirst ? FindSink.OfDouble.OP_FIND_FIRST : FindSink.OfDouble.OP_FIND_ANY;
>     }
> 
>     /**
> @@ -195,6 +193,14 @@
>             public Optional<T> get() {
>                 return hasValue ? Optional.of(value) : null;
>             }
> +
> +            static final TerminalOp<?, ?> OP_FIND_FIRST = new FindOp<>(true,
> +                    StreamShape.REFERENCE, Optional.empty(),
> +                    Optional::isPresent, FindSink.OfRef::new);
> +
> +            static final TerminalOp<?, ?> OP_FIND_ANY = new FindOp<>(false,
> +                    StreamShape.REFERENCE, Optional.empty(),
> +                    Optional::isPresent, FindSink.OfRef::new);
>         }
> 
>         /** Specialization of {@code FindSink} for int streams */
> @@ -210,6 +216,13 @@
>             public OptionalInt get() {
>                 return hasValue ? OptionalInt.of(value) : null;
>             }
> +
> +            static final TerminalOp<Integer, OptionalInt> OP_FIND_FIRST = new FindOp<>(true,
> +                    StreamShape.INT_VALUE, OptionalInt.empty(),
> +                    OptionalInt::isPresent, FindSink.OfInt::new);
> +            static final TerminalOp<Integer, OptionalInt> OP_FIND_ANY = new FindOp<>(false,
> +                    StreamShape.INT_VALUE, OptionalInt.empty(),
> +                    OptionalInt::isPresent, FindSink.OfInt::new);
>         }
> 
>         /** Specialization of {@code FindSink} for long streams */
> @@ -225,6 +238,13 @@
>             public OptionalLong get() {
>                 return hasValue ? OptionalLong.of(value) : null;
>             }
> +
> +            static final TerminalOp<Long, OptionalLong> OP_FIND_FIRST = new FindOp<>(true,
> +                    StreamShape.LONG_VALUE, OptionalLong.empty(),
> +                    OptionalLong::isPresent, FindSink.OfLong::new);
> +            static final TerminalOp<Long, OptionalLong> OP_FIND_ANY = new FindOp<>(false,
> +                    StreamShape.LONG_VALUE, OptionalLong.empty(),
> +                    OptionalLong::isPresent, FindSink.OfLong::new);
>         }
> 
>         /** Specialization of {@code FindSink} for double streams */
> @@ -240,6 +260,13 @@
>             public OptionalDouble get() {
>                 return hasValue ? OptionalDouble.of(value) : null;
>             }
> +
> +            static final TerminalOp<Double, OptionalDouble> OP_FIND_FIRST = new FindOp<>(true,
> +                    StreamShape.DOUBLE_VALUE, OptionalDouble.empty(),
> +                    OptionalDouble::isPresent, FindSink.OfDouble::new);
> +            static final TerminalOp<Double, OptionalDouble> OP_FIND_ANY = new FindOp<>(false,
> +                    StreamShape.DOUBLE_VALUE, OptionalDouble.empty(),
> +                    OptionalDouble::isPresent, FindSink.OfDouble::new);
>         }
>     }
> 
> -------
> 
> Ron