Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v23]

2026-01-14 Thread Doug Lea
On Mon, 12 Jan 2026 12:44:49 GMT, Viktor Klang  wrote:

>> Doug Lea has updated the pull request with a new target base due to a merge 
>> or a rebase. The incremental webrev excludes the unrelated changes brought 
>> in by the merge/rebase. The pull request contains 33 additional commits 
>> since the last revision:
>> 
>>  - Merge branch 'openjdk:master' into JDK-8373118
>>  - reunify push; improve contention vs activation vs park balance
>>  - Undo unrelated change
>>  - Re-introduce acquiring array reads; re-arrange to rely on volatile base 
>> index
>>  - Change signalWork fencing; in-progress activation changes
>>  - Merge branch 'openjdk:master' into JDK-8373118
>>  - Split external push
>>  - Undo/redo ordering changes
>>  - Strengthen some orderings
>>  - Merge branch 'openjdk:master' into JDK-8373118
>>  - ... and 23 more: https://git.openjdk.org/jdk/compare/8e9ec950...f42d2475
>
> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1830:
> 
>> 1828: int pc = parallelism, i, sp; // rely on caller sync for 
>> initial reads
>> 1829: long c = U.getLong(this, CTL);
>> 1830: WorkQueue[] qs = queues;
> 
> If we only read this on entry, doesn't that risk not observing the number of 
> queues growing?

No, see your other comment :-)

> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1854:
> 
>> 1852: break;
>> 1853: }
>> 1854: qs = queues;
> 
> Regarding https://github.com/openjdk/jdk/pull/28797/changes#r2682127585 , so 
> we're refresh-reading it here, relying on (at least) the 
> https://github.com/openjdk/jdk/pull/28797/changes#diff-e398beb49cd8d3e6c2f3a8ca8eee97172c57d7f88f3ccd8a3c704632cab32f5fR1844
>  load?

Right. doing iit here also allows qs read and src read to be unordered

-

PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2690261219
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2690259920


Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v23]

2026-01-12 Thread Viktor Klang
On Sun, 11 Jan 2026 17:11:38 GMT, Doug Lea  wrote:

>> Changes signal filtering to avoid possible starvation
>
> Doug Lea has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains 33 additional commits since 
> the last revision:
> 
>  - Merge branch 'openjdk:master' into JDK-8373118
>  - reunify push; improve contention vs activation vs park balance
>  - Undo unrelated change
>  - Re-introduce acquiring array reads; re-arrange to rely on volatile base 
> index
>  - Change signalWork fencing; in-progress activation changes
>  - Merge branch 'openjdk:master' into JDK-8373118
>  - Split external push
>  - Undo/redo ordering changes
>  - Strengthen some orderings
>  - Merge branch 'openjdk:master' into JDK-8373118
>  - ... and 23 more: https://git.openjdk.org/jdk/compare/af30d06a...f42d2475

src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1830:

> 1828: int pc = parallelism, i, sp; // rely on caller sync for initial 
> reads
> 1829: long c = U.getLong(this, CTL);
> 1830: WorkQueue[] qs = queues;

If we only read this on entry, doesn't that risk not observing the number of 
queues growing?

src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1854:

> 1852: break;
> 1853: }
> 1854: qs = queues;

Regarding https://github.com/openjdk/jdk/pull/28797/changes#r2682127585 , so 
we're refresh-reading it here, relying on (at least) the 
https://github.com/openjdk/jdk/pull/28797/changes#diff-e398beb49cd8d3e6c2f3a8ca8eee97172c57d7f88f3ccd8a3c704632cab32f5fR1844
 load?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2682127585
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2682133420


Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v23]

2026-01-12 Thread Viktor Klang
On Sun, 11 Jan 2026 17:11:38 GMT, Doug Lea  wrote:

>> Changes signal filtering to avoid possible starvation
>
> Doug Lea has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains 33 additional commits since 
> the last revision:
> 
>  - Merge branch 'openjdk:master' into JDK-8373118
>  - reunify push; improve contention vs activation vs park balance
>  - Undo unrelated change
>  - Re-introduce acquiring array reads; re-arrange to rely on volatile base 
> index
>  - Change signalWork fencing; in-progress activation changes
>  - Merge branch 'openjdk:master' into JDK-8373118
>  - Split external push
>  - Undo/redo ordering changes
>  - Strengthen some orderings
>  - Merge branch 'openjdk:master' into JDK-8373118
>  - ... and 23 more: https://git.openjdk.org/jdk/compare/849376f7...f42d2475

src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1260:

> 1258: U.putReferenceVolatile(a, slotOffset(m & s), task);
> 1259: if (unlock != 1)  // release external lock
> 1260: U.putInt(this, PHASE, unlock);

Doesn't this need to at least be a `putIntRelease`?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2681978895


Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v23]

2026-01-12 Thread Viktor Klang
On Sun, 11 Jan 2026 17:11:38 GMT, Doug Lea  wrote:

>> Changes signal filtering to avoid possible starvation
>
> Doug Lea has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains 33 additional commits since 
> the last revision:
> 
>  - Merge branch 'openjdk:master' into JDK-8373118
>  - reunify push; improve contention vs activation vs park balance
>  - Undo unrelated change
>  - Re-introduce acquiring array reads; re-arrange to rely on volatile base 
> index
>  - Change signalWork fencing; in-progress activation changes
>  - Merge branch 'openjdk:master' into JDK-8373118
>  - Split external push
>  - Undo/redo ordering changes
>  - Strengthen some orderings
>  - Merge branch 'openjdk:master' into JDK-8373118
>  - ... and 23 more: https://git.openjdk.org/jdk/compare/18d2870a...f42d2475

src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 2046:

> 2044: Thread.yield();  // reduce unproductive 
> scanning
> 2045: for (int s = SPIN_WAITS; (idle = w.phase & IDLE) != 
> 0 && --s != 0;)
> 2046: Thread.onSpinWait();

spinning _after_ yielding? 🤔

-

PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2681620777


Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v23]

2026-01-11 Thread Doug Lea
> Changes signal filtering to avoid possible starvation

Doug Lea has updated the pull request with a new target base due to a merge or 
a rebase. The incremental webrev excludes the unrelated changes brought in by 
the merge/rebase. The pull request contains 33 additional commits since the 
last revision:

 - Merge branch 'openjdk:master' into JDK-8373118
 - reunify push; improve contention vs activation vs park balance
 - Undo unrelated change
 - Re-introduce acquiring array reads; re-arrange to rely on volatile base index
 - Change signalWork fencing; in-progress activation changes
 - Merge branch 'openjdk:master' into JDK-8373118
 - Split external push
 - Undo/redo ordering changes
 - Strengthen some orderings
 - Merge branch 'openjdk:master' into JDK-8373118
 - ... and 23 more: https://git.openjdk.org/jdk/compare/efb906cb...f42d2475

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/28797/files
  - new: https://git.openjdk.org/jdk/pull/28797/files/d2b6c7c0..f42d2475

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=28797&range=22
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=28797&range=21-22

  Stats: 17279 lines in 337 files changed: 12042 ins; 3984 del; 1253 mod
  Patch: https://git.openjdk.org/jdk/pull/28797.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/28797/head:pull/28797

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