RFR: 8336707: Contention of ForkJoinPool grows when stealing works

[Doug Lea]

On Wed, 16 Oct 2024 16:32:32 GMT, Viktor Klang <vklang at openjdk.org> wrote:

>> This addresses tendencies in previous update to increase fencing, scanning, and signalling that can increase contention, and slow down performance especially on ARM platforms. It also uses more ARM-friendly constructions to reduce overhead (leading to several changes that all of the same form),
>
> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 854:
> 
>> 852:      * usages of ForkJoinTasks ignore interrupt status when executing
>> 853:      * or awaiting completion.  Otherwise, reporting task results or
>> 854:      * exceptions is preferred to throwing InterruptedExeptions,
> 
> Suggestion:
> 
>      * exceptions is preferred to throwing InterruptedExceptions,

Thanks.  I keep losing that typo fix!

> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1287:
> 
>> 1285:             if (!internal)
>> 1286:                 unlockPhase();
>> 1287:             if ((room == 0 || U.getReference(a, pk) == null) && pool != null)
> 
> We used to look at `- 2` but now we look at `- 1`, perhaps that could account for stalls?

Well, the interplay of signal rules in push, runWorker, and deactivate lead to under/over signalling, scanning, contention. I'm about to commit a different tactic.

> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1913:
> 
>> 1911:                 if (!all)
>> 1912:                     break;
>> 1913:             }
> 
> Stylistically, it might be cleaner to do this:
> 
> Suggestion:
> 
>         do {
>             WorkQueue[] qs; WorkQueue v; int sp, i;
>             if ((sp = (int)c) == 0 || (qs = queues) == null ||
>                 qs.length <= (i = sp & SMASK) || (v = qs[i]) == null)
>                 break;
>             if (c == (c = compareAndExchangeCtl(
>                           c, ((UMASK & (c + RC_UNIT)) | (c & TC_MASK) |
>                               (v.stackPred & LMASK))))) {
>                 v.phase = sp;
>                 if (v.parking != 0)
>                     U.unpark(v.owner);
>             } while(all);

Yeah, except not quite that way, since the break should occur only if the CAS succeeds. I'll try other options.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21507#discussion_r1806413628
PR Review Comment: https://git.openjdk.org/jdk/pull/21507#discussion_r1806417526
PR Review Comment: https://git.openjdk.org/jdk/pull/21507#discussion_r1806411821