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

Doug Lea dl at openjdk.org
Sun Jan 11 17:23:36 UTC 2026


On Thu, 8 Jan 2026 23:38:30 GMT, Viktor Klang <vklang at openjdk.org> wrote:

>> Doug Lea has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Undo unrelated change
>>  - Re-introduce acquiring array reads; re-arrange to rely on volatile base index
>
> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1261:
> 
>> 1259:                 if (U.getReferenceAcquire(a, slotOffset(m & (s - 1))) == null &&
>> 1260:                     pool != null)
>> 1261:                     pool.signalWork(this, s); // may have appeared empty
> 
> @DougLea I really like how push turned out here, it's arguably one of the most elegant versions of this method.

Too bad I had to re-uglify it a little!

> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 2063:
> 
>> 2061:         int idle = IDLE, phase;
>> 2062:         if ((runState & STOP) == 0L && w != null &&
>> 2063:              (idle = (phase = w.spinWaitPhase()) & IDLE) != 0) {
> 
> @DougLea Not that it'll happen frequently, but it _might_ be worth checking `(runState & STOP) == 0L` directly after the spinwait, and then switch from a `for(;;)` to a do-while where the while-condition is `(runState & STOP) == 0L`. That would avoid the small risk of needing to call setCurrentBlocker twice for situations where the pool transitions to STOP while the spinwait is running.
> 
> It'd be interesting to see if such a change has any discernable impact on ramp-down for benches that include startup and shutdown as a part of the benching process.

Thanks. Reworked in a slightly different way though.

> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 2281:
> 
>> 2279:                                 if (U.compareAndSetReference(a, k, t, null)) {
>> 2280:                                     q.base = b + 1;
>> 2281:                                     U.putIntVolatile(w, WorkQueue.SOURCE, j);
> 
> @DougLea Since WorkQueue::base is volatile with this PR, it _might_ be worth performing a plain or opaque write to q.base followed by the volatile write to WorkQueue.SOURCE? 🤔

Making it volatile introduced too many unnecessary ordering dependencies, so I undid that.

> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 2329:
> 
>> 2327:                         for (;;) {
>> 2328:                             ForkJoinTask<?> t; ForkJoinTask<?>[] a;
>> 2329:                             int b, cap, nb; long k;
> 
> @DougLea `nb` is never used.

fixed

> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 3083:
> 
>> 3081:     static ForkJoinPool asyncCommonPool() {
>> 3082:         ForkJoinPool cp; int p;
>> 3083:         if ((p = (cp = common).parallelism) == 0)
> 
> @DougLea `p` is never used.

fixed

> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 3859:
> 
>> 3857:      * granularities.The returned count does not include scheduled
>> 3858:      * tasks that are not yet ready to execute, which are reported
>> 3859:      * separately by method {@link getDelayedTaskCount}.
> 
> Suggestion:
> 
>      * separately by method {@link #getDelayedTaskCount}.

Thanks; fixed.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2679928842
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2679927714
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2679924965
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2679923069
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2679922712
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2679922306


More information about the core-libs-dev mailing list