-
Notifications
You must be signed in to change notification settings - Fork 6.1k
8367857: Implement JEP 525: Structured Concurrency (Sixth Preview) #27392
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
👋 Welcome back alanb! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@AlanBateman The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
|
/label remove compiler |
|
@AlanBateman |
src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java
Outdated
Show resolved
Hide resolved
src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java
Outdated
Show resolved
Hide resolved
src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java
Show resolved
Hide resolved
src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java
Outdated
Show resolved
Hide resolved
test/jdk/java/util/concurrent/StructuredTaskScope/StructuredTaskScopeTest.java
Show resolved
Hide resolved
test/jdk/java/util/concurrent/StructuredTaskScope/StructuredTaskScopeTest.java
Outdated
Show resolved
Hide resolved
Webrevs
|
|
I haven't read the complete javadoc, but this jumped out at me in the diff: Cancallation as the id and the multiple references to it. It must be a typo. |
The API uses "cancellation" and is not changed in this update. The id (used for links) is indeed "Cancallation". A pre-existing issue and not introduced in this update. So well spotted. We can fix this. |
|
I haven't been paying much attention to the recent previews of this feature, so feel free to point me to a relevant discussion elsewhere. As I understand it, an unchecked StructuredTaskScope.TimeoutException was added in JDK 25. There's also the well-known checked java.util.TimeoutException. I assume that the namesake exception was added solely because it's unchecked. I also assume UncheckedTimeoutException was considered at some stage, because in my mind, the situation is similar to that of IOException/UncheckedIOException. Why did you choose to add a namesake instead of an unchecked wrapper exception? Was it because you consider that the likelihood of mixing the two TimeoutException's is low in the context of a single file? |
It's better to bring API discussion to loom-dev, this PR is the refresh for JEP 525. A lot of time was spent in 2024 re-visiting all aspects of the API, including the use of checked exceptions. |
Thanks, done: https://mail.openjdk.org/pipermail/loom-dev/2025-October/007974.html |
src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java
Outdated
Show resolved
Hide resolved
src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java
Outdated
Show resolved
Hide resolved
src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java
Outdated
Show resolved
Hide resolved
| * successfully. Cancels the scope if any subtask fails. | ||
| */ | ||
| static final class AllSuccessful<T> implements Joiner<T, Stream<Subtask<T>>> { | ||
| static final class AllSuccessful<T> implements Joiner<T, List<T>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to clear() the results onTimeout before throwing the TimeoutException.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, Alan!
Updates for JEP 525.
Joiner:onTimeoutis added withjoinmethod changed to invoke this joiner method ifa timeout is configured and the timeout expires before or while waiting. The
onTimeoutthrows
TimeoutExceptionor may do nothing. This allows forJoinerimplementation thatare capable of returning a result from the subtasks that complete before the timeout expires.
configFunctionparameter to the 3-argopenis changed fromFunction<Configuration, Configuration>toUnaryOperator<Configuration>.Subtask::getandSubtask::exceptionchanged to consistently throw if called fromany thread before the scope owner has joined.
StructuredTaskScope::joinis now specified so that it may be called again if interrupted.Joiner::onForkandJoiner::onCompleteis changed fromSubtask<? extends TtoSubtask<T>.Joiner.allSuccessOrThrowis changed to return a list of results instead of a stream ofsubtasks.
Joiner.allUntilis changed to return a list of subtasks.Joiner.anySuccessfulResultOrThrowis renamed toanySuccessfulOrThrow.Joiner.allUntil(Predicate)is changed to allowjoinreturn the stream of all forkedsubtasks when the timeout expires.
Joineris no longer a@FunctionalInterface.Most of the changes are to API docs and tests. Some links are changed to use double hash mark.
Progress
Issues
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27392/head:pull/27392$ git checkout pull/27392Update a local copy of the PR:
$ git checkout pull/27392$ git pull https://git.openjdk.org/jdk.git pull/27392/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 27392View PR using the GUI difftool:
$ git pr show -t 27392Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27392.diff
Using Webrev
Link to Webrev Comment