RFR: 8347274: Gatherers.mapConcurrent exhibits undesired behavior under variable delays, interruption, and finishing [v2]

[Viktor Klang]

On Thu, 9 Jan 2025 15:23:18 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Viktor Klang has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Addresses PR review feedback
>
> src/java.base/share/classes/java/util/stream/Gatherers.java line 392:
> 
>> 390:                     while (proceed
>> 391:                         && (current = wip.peekFirst()) != null
>> 392:                         && (current.isDone() || atLeastN > 0)) {
> 
> It might be better to indent these two lines so that it's clearer what the while expression is vs. the code in the block.

Fair point—I reformatted that while clause to something which is hopefully easier to read.

> src/java.base/share/classes/java/util/stream/Gatherers.java line 421:
> 
>> 419:                     if (!success && !wip.isEmpty()) {
>> 420:                         // First signal cancellation for all tasks in progress
>> 421:                         for(var task : wip)
> 
> Minor formating nit is that you probably want a space in "for(", there are a few more of these in the patch.

Fixed!

> test/jdk/java/util/stream/GatherersMapConcurrentTest.java line 322:
> 
>> 320:     @ParameterizedTest
>> 321:     @MethodSource("concurrencyConfigurations")
>> 322:     public void behavesAsExpectedWhenCallerIsInterrupted(ConcurrencyConfig cc) {
> 
> It might be helpful for future maintainers to put a comment on the behaveAsExpectedXXX tests so that it's easier to figure out what they are testing.

That's fair, I decided to improve the names of the methods in the follow-on commit I just pushed 👍

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22999#discussion_r1909049038
PR Review Comment: https://git.openjdk.org/jdk/pull/22999#discussion_r1909048037
PR Review Comment: https://git.openjdk.org/jdk/pull/22999#discussion_r1909047753