On Wed, 7 May 2025 07:02:42 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> Changes for [JEP 505: Structured Concurrency (Fifth >> Preview)](https://openjdk.org/jeps/8340343). The proposal is to re-preview >> the API with some changes, specifically: >> >> - A >> [StructuredTaskScope](https://download.java.net/java/early_access/loom/docs/api/java.base/java/util/concurrent/StructuredTaskScope.html) >> is now opened with a static factory method instead of a constructor. Once >> opened, the API usage is unchanged: fork subtasks individually, join them as >> a unit, process outcome, and close. >> - In conjunction with moving to using a static open method, policy and >> desired outcome is now selected by specifying a Joiner to the open method >> rather than extending STS. A Joiner handles subtask completion and produces >> the result for join to return. Joiner.onComplete is the equivalent of >> overriding handleComplete previously. This change means that the subclasses >> ShutdownOnFailure and ShutdownOnSuccess are removed, replaced by factory >> methods on Joiner to get an equivalent Joiner. >> - The join method is changed to return the result or throw >> STS.FailedException, replacing the need for an API in subclasses to obtain >> the outcome. This removes the hazard that was forgetting to call >> throwIfFailed to propagate exceptions. >> - Configuration that was provided with parameters for the constructor is >> changed so that can be provided by a configuration function. >> - joinUntil is replaced by allowing a timeout be configured by the >> configuration function. This allows the timeout to apply the scope rather >> than the join method. >> >> The underlying implementation is unchanged except that ThreadFlock.shutdown >> and wakeup methods are no longer confined. The STS API implementation moves >> to non-public StructuedTaskScopeImpl because STS is now an interface. A >> non-public Joiners class is added with the built-in Joiner implementations. > > Alan Bateman has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 20 commits: > > - Merge branch 'master' into JDK-8342486 > - Merge branch 'master' into JDK-8342486 > - Merge branch 'master' into JDK-8342486 > - Sync up from loom repo > - Merge branch 'master' into JDK-8342486 > - Add JEP number, update copyright headers > - Merge branch 'master' into JDK-8342486 > - Sync up from loom repo > - Merge branch 'master' into JDK-8342486 > - Sync up from loom repo > - ... and 10 more: https://git.openjdk.org/jdk/compare/0eb680ca...5fec717f src/java.base/share/classes/java/util/concurrent/Joiners.java line 84: > 82: @SuppressWarnings("unchecked") > 83: var s = (Subtask<T>) subtask; > 84: subtasks.add(s); Suggestion: subtasks.add((Subtask<T>) subtask); The local variable s is only used once and can be simplified src/java.base/share/classes/java/util/concurrent/Joiners.java line 103: > 101: } else { > 102: return subtasks.stream(); > 103: } Suggestion: return subtasks.stream(); The else here is redundant src/java.base/share/classes/java/util/concurrent/Joiners.java line 155: > 153: case FAILED -> throw subtask.exception(); > 154: default -> throw new InternalError(); > 155: }; Suggestion: return switch (subtask.state()) { case SUCCESS -> subtask.get(); case FAILED -> throw subtask.exception(); default -> throw new InternalError(); }; The switch of the stateToInt method is aligned, so it should also be aligned here to keep the style consistent src/java.base/share/classes/java/util/concurrent/Joiners.java line 183: > 181: } else { > 182: return null; > 183: } Suggestion: if (ex != null) { throw ex; } return null; The else here is redundant src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java line 820: > 818: @PreviewFeature(feature = > PreviewFeature.Feature.STRUCTURED_CONCURRENCY) > 819: final class TimeoutException extends RuntimeException { > 820: @java.io.Serial Use import without full path ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21934#discussion_r2076962370 PR Review Comment: https://git.openjdk.org/jdk/pull/21934#discussion_r2076959693 PR Review Comment: https://git.openjdk.org/jdk/pull/21934#discussion_r2076964945 PR Review Comment: https://git.openjdk.org/jdk/pull/21934#discussion_r2076957675 PR Review Comment: https://git.openjdk.org/jdk/pull/21934#discussion_r2076968111
