RFR: 8306647: Implementation of Structured Concurrency (Preview)

[Alan Bateman]

On Sat, 20 May 2023 00:27:23 GMT, Paul Sandoz <psandoz 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?

Good catch, might have been the victim of search and replace, it should continue to link to ScopedValue.where.

> 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.

@theRealAph will want to comment on this as it is very performance sensitive. I think CallableAdapter.s is non-final to avoid the release fence.

> 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);

Good, avoids duplication.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13932#discussion_r1199577982
PR Review Comment: https://git.openjdk.org/jdk/pull/13932#discussion_r1200899135
PR Review Comment: https://git.openjdk.org/jdk/pull/13932#discussion_r1202097524