RFR: JDK-8319123 : Implementation of JEP-461: Stream Gatherers (Preview) [v2]

[Alan Bateman]

On Wed, 8 Nov 2023 21:08:03 GMT, Viktor Klang <vklang at openjdk.org> wrote:

>> This Pull-Request implements [JEP-461](https://openjdk.org/jeps/461)
>
> Viktor Klang has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Addressing review feedback
>  - Make Gatherer.andThen take a wildcard for the rhs Gatherer state type

src/java.base/share/classes/java/util/stream/Gatherers.java line 543:

> 541:      * @throws NullPointerException if any of the parameters are null
> 542:      */
> 543:     public static <T, R> Gatherer<T, ?, R> scan(

The examples (as code snippets) in the windowXXX and fold methods are great. It might help to add one to scan too as some readers might not know what prefix scan is, or they wonder if this is scan left or right.

src/java.base/share/classes/java/util/stream/Gatherers.java line 562:

> 560:     /**
> 561:      * An operation which executes operations concurrently
> 562:      * with a fixed window of max concurrency, using VirtualThreads.

Of the 5, this is the one that I'm not sure about so will be interesting to see if there is feedback from real world usage.

"VirtualThreads" isn't a class so maybe replace it with "virtual Threads", linked to Thread.html#virtual-threads.

src/java.base/share/classes/java/util/stream/Gatherers.java line 615:

> 613:                 assert wasAddedToWindow;
> 614: 
> 615:                 Thread.startVirtualThread(task);

At some point it might be worth seeing this should should use Executors.newVirtualThreadPerTaskExecutor(), that would at last simplify the cleanup as it would be executor.shutdownNow.  The serviceability benefit would be that an observe on a far hill taking a thread dump would have the virtual threads grouped for each mapConcurrent gatherer.

src/java.base/share/classes/java/util/stream/Stream.java line 1093:

> 1091:      *
> 1092:      * @implNote Implementations of this interface should provide their own
> 1093:      * implementation of this method.

I think the sentence goes in the implSpec as it tells implementers that they must override this method.

The `@see Gatherer` it probably not needed as the method already links to Gatherer.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1388083700
PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1388104543
PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1388101381
PR Review Comment: https://git.openjdk.org/jdk/pull/16420#discussion_r1388038457