RFR: 8306647: Implementation of Structured Concurrency (Preview)

[Paul Sandoz]

On Thu, 11 May 2023 13:08:55 GMT, Alan Bateman <alanb at openjdk.org> wrote:

> This is the implementation of:
> 
> - JEP 453: Structured Concurrency (Preview)
> - JEP 446: Scoped Values (Preview)
> 
> For the most part, this is just moving code and tests.  StructuredTaskScope moves to j.u.concurrent as a preview API, ScopedValue moves to j.lang as a preview API, and module jdk.incubator.concurrent has been removed. The significant API changes since incubator are:
> 
> - StructuredTaskScope.fork returns Subtask instead of Future (JEP 453 has a section on this)
> - ScopedValue.where methods are replaced with runWhere, callWhere and getWhere

src/java.base/share/classes/java/lang/ScopedValue.java line 252:

> 250:      * bound (or rebound) to {@code v1}, and {@code k2} bound (or rebound) to {@code v2}.
> 251:      * {@snippet lang=java :
> 252:      *     // @link substring="runWhere" target="#runWhere(ScopedValue, Object)" :

Is this correct?

src/java.base/share/classes/java/lang/ScopedValue.java line 399:

> 397:             var prevSnapshot = scopedValueBindings();
> 398:             var newSnapshot = new Snapshot(this, prevSnapshot);
> 399:             return runWith(newSnapshot, new CallableAdapter<R>(op));

Can we just do this instead?
Suggestion:

            return runWith(newSnapshot, op::get);

IIUC the current approach is to avoid the dynamic creation of a class via the invoke dynamic? I don't fully understand the comment about release fencing.

src/java.base/share/classes/java/lang/ScopedValue.java line 408:

> 406:         // runtime bytecode generation nor any release fencing.
> 407:         private static final class CallableAdapter<V> implements Callable<V> {
> 408:             private Supplier<? extends V> s;

Suggestion:

            private final Supplier<? extends V> s;

src/java.base/share/classes/java/lang/ScopedValue.java line 558:

> 556:      * This method is implemented to be equivalent to:
> 557:      * {@snippet lang=java :
> 558:      *     // @link substring="call" target="Carrier#call(Callable)" :

Suggestion:

     *     // @link substring="get" target="Carrier#get(Supplier)" :

?

src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java line 159:

> 157:  * The example uses {@link Supplier#get()} to get the result of each subtask. Using
> 158:  * {@code Supplier} instead of {@code Subtask} is preferred for common cases where the
> 159:  * the object returned by fork is only used to get the result of a subtask that completed

Suggestion:

 * {@code Supplier} instead of {@code Subtask} is preferred for common cases where
 * the object returned by fork is only used to get the result of a subtask that completed

src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java line 1077:

> 1075:             }
> 1076: 
> 1077:             throw new IllegalStateException("No completed subtasks");

I believe it may be possible to implement as the following if you so wish:
Suggestion:

            return result(ExecutionException::new);

src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java line 1251:

> 1249:             Throwable exception = firstException;
> 1250:             if (exception != null)
> 1251:                 throw new ExecutionException(exception);

Suggestion:

            throwIfFailed(ExecutionException::new);

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13932#discussion_r1199509233
PR Review Comment: https://git.openjdk.org/jdk/pull/13932#discussion_r1200863513
PR Review Comment: https://git.openjdk.org/jdk/pull/13932#discussion_r1199502950
PR Review Comment: https://git.openjdk.org/jdk/pull/13932#discussion_r1199508974
PR Review Comment: https://git.openjdk.org/jdk/pull/13932#discussion_r1200910221
PR Review Comment: https://git.openjdk.org/jdk/pull/13932#discussion_r1201014854
PR Review Comment: https://git.openjdk.org/jdk/pull/13932#discussion_r1201028382