On Fri, 12 Dec 2025 23:05:48 GMT, Viktor Klang <[email protected]> wrote:

>> Doug Lea has updated the pull request with a new target base due to a merge 
>> or a rebase. The incremental webrev excludes the unrelated changes brought 
>> in by the merge/rebase. The pull request contains five additional commits 
>> since the last revision:
>> 
>>  - Merge branch 'openjdk:master' into JDK-8373118
>>  - Address review comments
>>  - signalWork diagnostic
>>  - filter by index
>>  - For testing
>
> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1468:
> 
>> 1466:                     ForkJoinTask<?> t; int qcap; long qk;
>> 1467:                     ForkJoinTask<?>[] qa = q.array;
>> 1468:                     if (q.base != qbase || qa == null || (qcap = 
>> qa.length) <= 0 ||
> 
> Seems like a negative array length would be problematic in general, so 
> simplifying to a 0-check here?
> 
> Suggestion:
> 
>                     if (q.base != qbase || qa == null || (qcap = qa.length) 
> == 0 ||

C1 used to not know that lengths are nonnegative, so I always do it this way. 
Probably needlessly but doesn't hurt.

> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1471:
> 
>> 1469:                         (t = (ForkJoinTask<?>)U.getReferenceAcquire(
>> 1470:                             qa, qk = slotOffset((qcap - 1) & qbase))) 
>> == null ||
>> 1471:                         q.base != qbase ||
> 
> Is this (repeated) check intended? (i.e. are we worried about q.base drift 
> between polling the array?)

Yes, needed (everywhere) to avoid bad CASes in rare cases (like array 
wraparound).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2616363243
PR Review Comment: https://git.openjdk.org/jdk/pull/28797#discussion_r2616364228

Reply via email to