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