On Thu, 8 Jan 2026 23:38:30 GMT, Viktor Klang <[email protected]> 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
