On Thu, 11 May 2023 13:08:55 GMT, Alan Bateman <al...@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
