Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v21]
On Thu, 8 Jan 2026 16:47:08 GMT, Viktor Klang wrote:
>> Doug Lea has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Change signalWork fencing; in-progress activation changes
>
> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1830:
>
>> 1828: */
>> 1829: final void signalWork(WorkQueue q, int qbase) {
>> 1830: int pc = U.getIntAcquire(this, PARALLELISM);
>
> I like this, as this has the nice benefit of seeing potential changes to
> `parallelism` sooner.
It's now back to being fresh upon entry, but not upon retries, mainly because
retires are now less frequent.
> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1855:
>
>> 1853: break;
>> 1854: if (c == (c = ctl) &&
>> 1855: c == (c = U.compareAndExchangeLong(this, CTL, c, nc)))
>> {
>
> Are there any measurable differences between the above and `c == (c = ctl) &&
> U.compareAndSetLong(this, CTL, c, nc)` or `c ==
> U.compareAndExchangeLong(this, CTL, c, nc)`? 🤔
It depends on whether there is much contention and/or much filtering. I redid
some of it to take a middle ground on this, and results seems almost always
better. (Which made me realized that I should reconsider a few other
constructions elsewhere.)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2679952355
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2679948199
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v21]
On Thu, 8 Jan 2026 23:22:30 GMT, Viktor Klang wrote: >> That loop rereads q.array on each iteration, which means it is never stale. >> It's possible ito nstead check for staleness and rescan if so. I just tried >> a version with this, but it's not looking any better. > > Yeah, I was just thinking that the `array`-field is non-volatile so a read > isn't guaranteed to yield anything new unless that read is piggybacking on > some other fence. The t = U.getReferenceAcquire(...) is doing a lot of work here. Every read after it must be freshly accessed. Thanks for the implicit reminder though that I should do almost all those subsequent reads until CAS at once. - PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2679934255
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v21]
On Thu, 8 Jan 2026 18:35:03 GMT, Doug Lea wrote:
>> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1961:
>>
>>> 1959: if (q.base == b) {// else
>>> inconsistent
>>> 1960: if (t == null) {
>>> 1961: if (q.array == a) { // else
>>> resized
>>
>> @DougLea Do we have any good sense of how "far behind" a completed resize a
>> worker can end up being (i.e. looking at an array that has been replaced
>> already)? I'm just thinking if the rescanning is spending cycles looking at
>> the wrong thing.
>
> That loop rereads q.array on each iteration, which means it is never stale.
> It's possible ito nstead check for staleness and rescan if so. I just tried a
> version with this, but it's not looking any better.
Yeah, I was just thinking that the `array`-field is non-volatile so a read
isn't guaranteed to yield anything new unless that read is piggybacking on some
other fence.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2674233841
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v21]
On Thu, 8 Jan 2026 16:51:34 GMT, Viktor Klang wrote:
>> Doug Lea has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Change signalWork fencing; in-progress activation changes
>
> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1961:
>
>> 1959: if (q.base == b) {// else
>> inconsistent
>> 1960: if (t == null) {
>> 1961: if (q.array == a) { // else
>> resized
>
> @DougLea Do we have any good sense of how "far behind" a completed resize a
> worker can end up being (i.e. looking at an array that has been replaced
> already)? I'm just thinking if the rescanning is spending cycles looking at
> the wrong thing.
That loop rereads q.array on each iteration, which means it is never stale.
It's possible ito nstead check for staleness and rescan if so. I just tried a
version with this, but it's not looking any better.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2673445481
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v21]
On Thu, 8 Jan 2026 15:18:00 GMT, Doug Lea wrote:
>> Changes signal filtering to avoid possible starvation
>
> Doug Lea has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Change signalWork fencing; in-progress activation changes
src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1961:
> 1959: if (q.base == b) {// else
> inconsistent
> 1960: if (t == null) {
> 1961: if (q.array == a) { // else
> resized
@DougLea Do we have any good sense of how "far behind" a completed resize a
worker can end up being (i.e. looking at an array that has been replaced
already)? I'm just thinking if the rescanning is spending cycles looking at the
wrong thing.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2673120173
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v21]
On Thu, 8 Jan 2026 15:18:00 GMT, Doug Lea wrote:
>> Changes signal filtering to avoid possible starvation
>
> Doug Lea has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Change signalWork fencing; in-progress activation changes
src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1830:
> 1828: */
> 1829: final void signalWork(WorkQueue q, int qbase) {
> 1830: int pc = U.getIntAcquire(this, PARALLELISM);
I like this, as this has the nice benefit of seeing potential changes to
`parallelism` sooner.
src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1855:
> 1853: break;
> 1854: if (c == (c = ctl) &&
> 1855: c == (c = U.compareAndExchangeLong(this, CTL, c, nc))) {
Are there any measurable differences between the above and `c == (c = ctl) &&
U.compareAndSetLong(this, CTL, c, nc)` or `c == U.compareAndExchangeLong(this,
CTL, c, nc)`? 🤔
-
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2673104063
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2673100883
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v21]
On Thu, 8 Jan 2026 16:08:41 GMT, Viktor Klang wrote: >> Doug Lea has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Change signalWork fencing; in-progress activation changes > > src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1268: > >> 1266: * Resizes the queue array and pushes unless out of memory. >> 1267: * @param task the task; caller must ensure nonnull >> 1268: * @param pool the pool to signal upon resize > > @DougLea So this param now becomes "the pool to signal upon resize, if null > and the queue's owner has a pool then that pool will be used for the signal > instead" Yes. I'll fix the param spec (among other upcoming docs fixes when this settles.) > src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1295: > >> 1293: ForkJoinWorkerThread o; >> 1294: if (pool != null || >> 1295: ((o = owner) != null && (pool = o.pool) != >> null)) > > Ok, so now you can't intentionally skip a signal by passing `null` as a pool. > Might be for the best, since we always want to signal if we deem it to be > needed. Right. This covers usages of lazySubmit that we don't think can occur in loom, but now there as a safeguard. - PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2673020403 PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2673015766
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v21]
On Thu, 8 Jan 2026 15:18:00 GMT, Doug Lea wrote: >> Changes signal filtering to avoid possible starvation > > Doug Lea has updated the pull request incrementally with one additional > commit since the last revision: > > Change signalWork fencing; in-progress activation changes src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1268: > 1266: * Resizes the queue array and pushes unless out of memory. > 1267: * @param task the task; caller must ensure nonnull > 1268: * @param pool the pool to signal upon resize @DougLea So this param now becomes "the pool to signal upon resize, if null and the queue's owner has a pool then that pool will be used for the signal instead" src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1295: > 1293: ForkJoinWorkerThread o; > 1294: if (pool != null || > 1295: ((o = owner) != null && (pool = o.pool) != > null)) Ok, so now you can't intentionally skip a signal by passing `null` as a pool. Might be for the best, since we always want to signal if we deem it to be needed. - PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2672970727 PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2672974508
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v21]
> Changes signal filtering to avoid possible starvation Doug Lea has updated the pull request incrementally with one additional commit since the last revision: Change signalWork fencing; in-progress activation changes - Changes: - all: https://git.openjdk.org/jdk/pull/28797/files - new: https://git.openjdk.org/jdk/pull/28797/files/54a8672a..b0d99c2f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=28797&range=20 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=28797&range=19-20 Stats: 131 lines in 1 file changed: 36 ins; 42 del; 53 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
