On Sat, 20 May 2023 00:27:23 GMT, Paul Sandoz <[email protected]> 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
