Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v26]
On Thu, 15 Jan 2026 14:29:02 GMT, Viktor Klang wrote:
>> Doug Lea has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Another set of contend vs deactivate vs park tradeoffs
>
> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 2593:
>
>> 2591: break;
>> 2592: if ((q = qs[i = (id = r & EXTERNAL_ID_MASK) & (n - 1)]) ==
>> null) {
>> 2593: WorkQueue newq = new WorkQueue(null, id, 0, false);
>
> @DougLea Is there a specific reason as to why CLEAR_TLS is false here?
Because external queues don't have owners, so it would never apply.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2779782366
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v26]
On Thu, 15 Jan 2026 12:39:43 GMT, Viktor Klang wrote:
>> Doug Lea has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Another set of contend vs deactivate vs park tradeoffs
>
> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1861:
>
>> 1859: break;
>> 1860: }
>> 1861: }
>
> The following might be a potential alternative encoding of signalWork—it
> would be interesting to hear if it makes any difference on your testing
> array, Doug:
>
>
> final void signalWork(WorkQueue src, int base) {
> int pc = parallelism, i, sp; // rely on caller sync for initial reads
> long c = U.getLong(this, CTL);
> WorkQueue[] qs;
> while ((short)(c >>> RC_SHIFT) < pc && (qs = queues) != null &&
>qs.length > (i = (sp = (int)c) & SMASK) && (src == null ||
> src.base - base < 1)) {
> if (i == 0) {
> if ((short)(c >>> TC_SHIFT) >= pc)
> break;
> if (c == (c = U.compareAndExchangeLong(this, CTL, c, ((c +
> TC_UNIT) & TC_MASK) | ((c + RC_UNIT) & RC_MASK {
> createWorker();
> break;
> }
> }
> else {
> WorkQueue v;
> if ((v = qs[i]) == null)
> break;
> if (c == (c = U.compareAndExchangeLong(this, CTL, c,
> (v.stackPred & LMASK) | ((c + RC_UNIT) & UMASK {
> v.phase = sp;
> if (v.parking != 0)
> U.unpark(v.owner);
> break;
> }
> }
> }
> }
Thanks for prodding me to instrument current version. I saw that retries are no
longer very common -- max seen across various contention-prone tests was 17,
but in most never more than 4 except more during warmup before code is
compiled. Versions that did filter check before CAS systematically have more
retries though. And as I noticed in some previous versions, separating the
branches with different CAes invites the compiler to rearrange code in a worse
way to only have one CAS. at least in the surprisingly common case where it
inlines signalWork in callers. So net change from all this so far was a couple
of cosmetic tweaks.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2702638385
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v26]
On Wed, 14 Jan 2026 12:41:50 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: > > Another set of contend vs deactivate vs park tradeoffs src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1980: > 1978: signalWork(q, nb); > 1979: rescans = 1; > 1980: if (taken++ == 0 || src != qid) or `++taken == 1` - PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2702013524
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v26]
On Wed, 14 Jan 2026 12:41:50 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:
>
> Another set of contend vs deactivate vs park tradeoffs
src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 2593:
> 2591: break;
> 2592: if ((q = qs[i = (id = r & EXTERNAL_ID_MASK) & (n - 1)]) ==
> null) {
> 2593: WorkQueue newq = new WorkQueue(null, id, 0, false);
@DougLea Is there a specific reason as to why CLEAR_TLS is false here?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2694612904
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v26]
On Wed, 14 Jan 2026 12:41:50 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:
>
> Another set of contend vs deactivate vs park tradeoffs
src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1267:
> 1265: pool.signalWork(this, s); // may have appeared empty
> 1266: }
> 1267: }
Not sure if it helps in practice, but we could elide the getReferenceAcquire in
certain cases while still ensuring that its performed prior to the unlock with
something like:
final void push(ForkJoinTask task, ForkJoinPool pool, int unlock) {
ForkJoinTask[] a = array;
int b = base, s = top, cap, m;
if (a == null || (cap = a.length) <= s + 1 - b || (m = cap - 1) < 0)
growAndPush(task, pool, unlock);
else {
top = s + 1;
U.getAndSetReference(a, slotOffset(m & s), task);
if (pool != null && U.getReferenceAcquire(a, slotOffset(m & (s
- 1))) == null)
pool = null;
if (unlock != 1) {// release external lock
U.putInt(this, PHASE, unlock);
U.storeFence();
}
if (pool != null)
pool.signalWork(this, s); // may have appeared empty
}
}
-
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2694290976
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v26]
On Wed, 14 Jan 2026 12:41:50 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:
>
> Another set of contend vs deactivate vs park tradeoffs
src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1861:
> 1859: break;
> 1860: }
> 1861: }
The following might be a potential alternative encoding of signalWork—it would
be interesting to hear if it makes any difference on your testing array, Doug:
final void signalWork(WorkQueue src, int base) {
int pc = parallelism, i, sp; // rely on caller sync for initial reads
long c = U.getLong(this, CTL);
WorkQueue[] qs;
while ((short)(c >>> RC_SHIFT) < pc && (qs = queues) != null &&
qs.length > (i = (sp = (int)c) & SMASK) && (src == null ||
src.base - base < 1)) {
if (i == 0) {
if ((short)(c >>> TC_SHIFT) >= pc)
break;
if (c == (c = U.compareAndExchangeLong(this, CTL, c, ((c +
TC_UNIT) & TC_MASK) | ((c + RC_UNIT) & RC_MASK {
createWorker();
break;
}
}
else {
WorkQueue v;
if ((v = qs[i]) == null)
break;
if (c == (c = U.compareAndExchangeLong(this, CTL, c,
(v.stackPred & LMASK) | ((c + RC_UNIT) & UMASK {
v.phase = sp;
if (v.parking != 0)
U.unpark(v.owner);
break;
}
}
}
}
-
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2694226850
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v26]
On Wed, 14 Jan 2026 13:01:14 GMT, Viktor Klang wrote: >> Doug Lea has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Another set of contend vs deactivate vs park tradeoffs > > src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1988: > >> 1986: } >> 1987: } >> 1988: if (rescans > 0) > > If avoiding arithmetic turns out successful, we might just introduce `static > final` constants and treat this as a regular state machine :) Ideally, runWorker would take its original form: while(scanState != stop) scanState = scan(scanState, ...); But there are too many bits of state, and sometimes-slow encoding/decoding to do this. But I'll look into some compromises. - PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2690538961
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v26]
On Wed, 14 Jan 2026 12:41:50 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: > > Another set of contend vs deactivate vs park tradeoffs src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1988: > 1986: } > 1987: } > 1988: if (rescans > 0) If avoiding arithmetic turns out successful, we might just introduce `static final` constants and treat this as a regular state machine :) - PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2690340971
Re: RFR: 8373118: Test java/lang/Thread/virtual/Starvation.java timed out [v26]
> Changes signal filtering to avoid possible starvation Doug Lea has updated the pull request incrementally with one additional commit since the last revision: Another set of contend vs deactivate vs park tradeoffs - Changes: - all: https://git.openjdk.org/jdk/pull/28797/files - new: https://git.openjdk.org/jdk/pull/28797/files/bb492a04..88f1466d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=28797&range=25 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=28797&range=24-25 Stats: 122 lines in 1 file changed: 49 ins; 41 del; 32 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
