Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v27]
On Mon, 19 Jan 2026 21:03:02 GMT, Viktor Klang wrote:
>> Doug Lea has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Simplify scan mode control by moving and reworking topLevelExec and
>> throwing on trim
>
> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1271:
>
>> 1269: }
>> 1270: if (pred == null && pool != null)
>> 1271: pool.signalWork(this, s); // may have appeared
>> empty
>
> @DougLea I presume we don't need to do the getReferenceAcquire if the pool is
> null?
>
> If so, then we could do something like:
>
>
> boolean signal = pool != null && U.getReferenceAcquire(a,
> slotOffset(m & (s - 1))) == null;
> if (unlock != 1) {// release external lock
> U.putInt(this, PHASE, unlock);
> U.storeFence();
> }
> if (signal)
> pool.signalWork(this, s); // may have appeared empty
I was skeptical about specializing around here, but after reading this, I found
that avoiding getAcquire when already known empty was s small win.
> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1303:
>
>> 1301: if (unlock != 1)
>> 1302: phase = unlock;
>> 1303: if (pool != null)
>
> @DougLea Are we 100% sure we always need to signal upon resize?
Yes, because worker in runWorker won't see a task and so won't propagate (and
it costs more to check this than just always doing it on resize).
-
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2779750895
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2779755766
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v27]
On Sun, 18 Jan 2026 21:07:48 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:
>
> Simplify scan mode control by moving and reworking topLevelExec and
> throwing on trim
src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1299:
> 1297: break;// lost to pollers
> 1298: newArray[k & newMask] = u;
> 1299: }
@DougLea Not sure if it helps loop unrolling, but a possible change might be
something like:
for (int k = s - 1, j = cap;
j > 0 && (u = (ForkJoinTask)U.getAndSetReference(a,
slotOffset(k & mask), null)) != null; // lost to pollers
--j, --k) {
newArray[k & newMask] = u;
}
src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1303:
> 1301: if (unlock != 1)
> 1302: phase = unlock;
> 1303: if (pool != null)
@DougLea Are we 100% sure we always need to signal upon resize?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2706072253
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2706073001
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v27]
On Sun, 18 Jan 2026 21:07:48 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:
>
> Simplify scan mode control by moving and reworking topLevelExec and
> throwing on trim
src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1271:
> 1269: }
> 1270: if (pred == null && pool != null)
> 1271: pool.signalWork(this, s); // may have appeared empty
@DougLea I presume we don't need to do the getReferenceAcquire if the pool is
null?
If so, then we could do something like:
boolean signal = pool != null && U.getReferenceAcquire(a,
slotOffset(m & (s - 1))) == null;
if (unlock != 1) {// release external lock
U.putInt(this, PHASE, unlock);
U.storeFence();
}
if (signal)
pool.signalWork(this, s); // may have appeared empty
-
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2706064195
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v27]
On Sun, 18 Jan 2026 21:07:48 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:
>
> Simplify scan mode control by moving and reworking topLevelExec and
> throwing on trim
src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 2059:
> 2057: if (idle != 0 && (runState & STOP) == 0L) {
> 2058: int noise = (phase ^ (phase >>> 16)) & (SPIN_WAITS - 1);
> 2059: int spins = (SPIN_WAITS << 1) | noise;
@DougLea Haha, it's a funny coincidence, because just last night I was thinking
we should try a randomized backoff, and now I saw this. :)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2702938962
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v27]
On Sun, 18 Jan 2026 21:07:48 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:
>
> Simplify scan mode control by moving and reworking topLevelExec and
> throwing on trim
src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1175:
> 1173: */
> 1174: @SuppressWarnings("serial")
> 1175: static final class WorkerTrimmedException extends RuntimeException
> { }
I presume that the intent is not to have this surface outside of FJP—might make
sense to create a cached instance to make it less risky to throw (cheaper to
construct + no risk of it triggering OOME under heavy load scenarios?)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2702927621
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v27]
On Sun, 18 Jan 2026 21:07:48 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: > > Simplify scan mode control by moving and reworking topLevelExec and > throwing on trim src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 2008: > 2006: a, bp = slotOffset((cap - 1) & (b = q.base))); > 2007: Object nt = U.getReference( > 2008: a, np = slotOffset((cap - 1) & (nb = b + 1))); @DougLea `nt` isn't used until it is overwritten by `nt = U.getReference(a, np);` I presume this is not intentional? - PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2702922891
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v27]
On Sun, 18 Jan 2026 21:07:48 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:
>
> Simplify scan mode control by moving and reworking topLevelExec and
> throwing on trim
src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1950:
> 1948: if (q.array == a && q.base == b &&
> 1949: U.getReference(a, bp) == t) {
> 1950: if (t == null) {
@DougLea Does it make any difference if we only confirm `t` in case of null,
and if that now shows anything different, we can use that for the later CAS?
if (q.array == a && q.base == b) {
if (t == null && (t =
(ForkJoinTask)U.getReference(a, bp)) == null) {
-
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2702907317
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v27]
> Changes signal filtering to avoid possible starvation Doug Lea has updated the pull request incrementally with one additional commit since the last revision: Simplify scan mode control by moving and reworking topLevelExec and throwing on trim - Changes: - all: https://git.openjdk.org/jdk/pull/28797/files - new: https://git.openjdk.org/jdk/pull/28797/files/88f1466d..a1e5ce94 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=28797&range=26 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=28797&range=25-26 Stats: 192 lines in 1 file changed: 57 ins; 60 del; 75 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
