RFR: 8336707: Contention of ForkJoinPool grows when stealing works [v24]

[Doug Lea]

On Tue, 12 Nov 2024 14:14:18 GMT, Viktor Klang <vklang at openjdk.org> wrote:

>> Doug Lea has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Address review comments
>
> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1870:
> 
>> 1868:         else if ((phase = w.phase) != 0 && (phase & IDLE) != 0)
>> 1869:             releaseWaiters();              // ensure released
>> 1870:         if (w == null || w.source != DROPPED) {
> 
> @DougLea If this isn't intended to be an `else-if` I'd recommend adding a line of whitespace between it and the line above.

Thanks for the prod. While making the control flow here read better, I noticed that adding a leading runState check in registerWorker further reduced overhead a bit when shutdown occurs while still ramping up.

> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1877:
> 
>> 1875:                                        (LMASK & c)))));
>> 1876:         }
>> 1877:         if (phase != 0 && w != null && (runState & STOP) == 0L) {
> 
> @DougLea If this isn't intended to be an else-if I'd recommend adding a line of whitespace between it and the line above.

now no more elses so less confuring

> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 2078:
> 
>> 2076:         w.phase = p;
>> 2077:         if (!compareAndSetCtl(pc, qc))        // try to enqueue
>> 2078:             return w.phase = phase;           // back out on possible signal
> 
> @DougLea We don't back out the stackPred change here, is that still valid? 🤔

yes. it's as if the deactivation didn't happen, so no need to increment version tag bits.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21507#discussion_r1838807759
PR Review Comment: https://git.openjdk.org/jdk/pull/21507#discussion_r1838808625
PR Review Comment: https://git.openjdk.org/jdk/pull/21507#discussion_r1838810950