RFR: Minor changes to `StructuredTaskScope` [v3]

Alan Bateman alanb at openjdk.java.net
Wed Apr 13 15:19:44 UTC 2022


On Tue, 12 Apr 2022 17:06:02 GMT, Paul Sandoz <psandoz at openjdk.org> wrote:

>> Some suggested minor changes to `StructuredTaskScope` source while reviewing the code.
>> 
>> Overall I like how this approach has boiled down to about as simple as it can get while supporting the constraints of structured concurrency (thanks in no small part to `ThreadFlock`).
>> 
>> I think it should be possible to simplify the subclass implementations if the values of `Future.State` were comparable as in `RUNNING` < `CANCELED` < `FAILED` < `SUCCESS`. Otherwise, introducing that internally could also simplify and avoid the task scopes holding on unnecessarily to futures that are not operated on after join. I was reluctant to make such a change here but could propose as a separate PR.
>> 
>> I was uncertain if the `ShutdownOnSuccess.result` methods and the various exception returning/operating methods on `ShutdownOnFailure` should be constrained to throw if called before `join` has completed. Otherwise, we should specify that if operated on before `join` has completed then the results are unspecified.
>> (There might be a simple way for join to return something other than `this`, thereby avoiding this issue, with some additional complexity, but I suspect you have already thought of that and took the minimal route.)
>> 
>> I presume there is an implicit happens-before edge on `join` that we can make explicit and specify?
>
> Paul Sandoz has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Memory consistency effects.

src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java line 256:

> 254:  * <h2>Memory consistency effects</h2>
> 255:  * <p>Actions in the owner thread of, or a thread contained in, the task scope prior to
> 256:  * {@linkplain #fork forking} of a {@code Runnable} or {@code Callable} task

The changes look fine, just one comment on this section is that there isn't an overload of fork that takes a Runnable, it's just Callable for now.

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

PR: https://git.openjdk.java.net/loom/pull/142


More information about the loom-dev mailing list