RFR: 8342486: Implement JEP 505: Structured Concurrency (Fifth Preview) [v8]

[Alan Bateman]

On Tue, 15 Apr 2025 13:25:41 GMT, Viktor Klang <vklang at openjdk.org> wrote:

>> Alan Bateman has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 15 commits:
>> 
>>  - Add JEP number, update copyright headers
>>  - Merge branch 'master' into JDK-8342486
>>  - Sync up from loom repo
>>  - Merge branch 'master' into JDK-8342486
>>  - Sync up from loom repo
>>  - Merge branch 'master' into JDK-8342486
>>  - Merge branch 'master' into JDK-8342486
>>  - Fix link
>>  - Merge branch 'master' into JDK-8342486
>>  - Sync up impl/tests form loom repo
>>  - ... and 5 more: https://git.openjdk.org/jdk/compare/3090e218...418bc3d3
>
> src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java line 496:
> 
>> 494:      * processed. It should be clear what the {@code Joiner} does vs. the application
>> 495:      * code at the use-site. In general, the {@code Joiner} implementation is not the
>> 496:      * place to code "business logic". A {@code Joiner} should be designed to be as
> 
> Suggestion:
> 
>      * place for "business logic". A {@code Joiner} should be designed to be as

Okay

> src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java line 563:
> 
>> 561:          * <p> In normal usage, this method will be called at most once by the {@code join}
>> 562:          * method to produce the result (or exception). The behavior of this method when
>> 563:          * invoked directly, and invoked more than once, is not specified. Where possible,
> 
> Suggestion:
> 
>          * invoked directly, and invoked more than once, is undefined. Where possible,

Okay

> src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java line 778:
> 
>> 776:         /**
>> 777:          * {@return a new {@code Configuration} object with the given timeout}
>> 778:          * The other components are the same as this object.
> 
> Suggestion:
> 
>          * The other other configuration options are preserved.

The class introduces them as components of the configuration rather than "options" so I think I'll leave this as is.

> src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java line 806:
> 
>> 804:          * @param  cause the cause, can be {@code null}
>> 805:          */
>> 806:         public FailedException(Throwable cause) {
> 
> Should this be public?

That's a good question, there is no reason for these constructors to be public.

> src/java.base/share/classes/java/util/concurrent/StructuredTaskScopeImpl.java line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
> 
> No changes in 2025?

It was created in 2024, hasn't had any changes until the rename from Config -> Configuration.

> src/java.base/share/classes/java/util/concurrent/StructuredTaskScopeImpl.java line 424:
> 
>> 422:      * Used to schedule a task to cancel the scope when a timeout expires.
>> 423:      */
>> 424:     private static class TimerSupport {
> 
> Might make sense to use the commonPool():s DelayScheduler?

TimerSupport hasn't changed, and pre-dates the FJP work to support delayed tasks, so maybe now is the time to migrate it.

> test/jdk/java/util/concurrent/StructuredTaskScope/StructuredTaskScopeTest.java line 1394:
> 
>> 1392:             assertEquals(2, subtasks.size());
>> 1393:             assertTrue(subtasks.get(0) == subtask1);
>> 1394:             assertTrue(subtasks.get(1) == subtask2);
> 
> Stylistic, there's an "assertSame" to assert that two references refer to the same instance.

Yes, indeed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21934#discussion_r2046135423
PR Review Comment: https://git.openjdk.org/jdk/pull/21934#discussion_r2046135526
PR Review Comment: https://git.openjdk.org/jdk/pull/21934#discussion_r2046136931
PR Review Comment: https://git.openjdk.org/jdk/pull/21934#discussion_r2046137297
PR Review Comment: https://git.openjdk.org/jdk/pull/21934#discussion_r2046138621
PR Review Comment: https://git.openjdk.org/jdk/pull/21934#discussion_r2046139774
PR Review Comment: https://git.openjdk.org/jdk/pull/21934#discussion_r2046144074