On Wed, 16 Oct 2024 16:32:32 GMT, Viktor Klang <vkl...@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
