Re: RFR: 8306647: Implementation of Structured Concurrency (Preview) [v4]

2023-06-05 Thread Mandy Chung
On Thu, 1 Jun 2023 13:43:33 GMT, Alan Bateman  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
>
> Alan Bateman has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 15 commits:
> 
>  - Sync up from loom repo
>  - Merge
>  - Sync with loom repo, re-work ScopedValue class description
>  - Sync up from loom repo
>  - Remove csm.Threads
>  - Merge
>  - Test should not be in update for main line
>  - Sync with loom repo
>  - Sync up tests frmo loom repo
>  - Sync up with loom repo
>  - ... and 5 more: https://git.openjdk.org/jdk/compare/a46b5acc...cc902ce6

I reviewed the implementation changes to promote an incubating API to a preview 
API.  That part looks good.

-

Marked as reviewed by mchung (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13932#pullrequestreview-1463317525


Re: RFR: 8306647: Implementation of Structured Concurrency (Preview) [v4]

2023-06-03 Thread Daniel Fuchs
On Thu, 1 Jun 2023 13:43:33 GMT, Alan Bateman  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
>
> Alan Bateman has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 15 commits:
> 
>  - Sync up from loom repo
>  - Merge
>  - Sync with loom repo, re-work ScopedValue class description
>  - Sync up from loom repo
>  - Remove csm.Threads
>  - Merge
>  - Test should not be in update for main line
>  - Sync with loom repo
>  - Sync up tests frmo loom repo
>  - Sync up with loom repo
>  - ... and 5 more: https://git.openjdk.org/jdk/compare/a46b5acc...cc902ce6

I reviewed the API documentation of `ScopedValue` (mostly the class level API 
documentation, but I had a look at the methods API too) and it looks good. It 
reads well, and when I had question they were usually answered in the next 
paragraph that followed what I was reading.

I didn't look at Structured Concurrency.

-

Marked as reviewed by dfuchs (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13932#pullrequestreview-1459172232


Re: RFR: 8306647: Implementation of Structured Concurrency (Preview) [v4]

2023-06-01 Thread Alan Bateman
> 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

Alan Bateman has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains 15 commits:

 - Sync up from loom repo
 - Merge
 - Sync with loom repo, re-work ScopedValue class description
 - Sync up from loom repo
 - Remove csm.Threads
 - Merge
 - Test should not be in update for main line
 - Sync with loom repo
 - Sync up tests frmo loom repo
 - Sync up with loom repo
 - ... and 5 more: https://git.openjdk.org/jdk/compare/a46b5acc...cc902ce6

-

Changes: https://git.openjdk.org/jdk/pull/13932/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=13932=03
  Stats: 9267 lines in 40 files changed: 4880 ins; 4325 del; 62 mod
  Patch: https://git.openjdk.org/jdk/pull/13932.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/13932/head:pull/13932

PR: https://git.openjdk.org/jdk/pull/13932