On Thu, 2 May 2024 21:47:50 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> expEnteringCount/expWaitingCount contain the tested patterns. I don't see
>> why they can't just replace
>> NUMBER_OF_ENTERING_THREADS/NUMBER_OF_WAITING_THREADS in the comments also.
>> In fact it is confusing if you don't because code right below the comments
>> references expEnteringCount/expWaitingCount, not
>> NUMBER_OF_ENTERING_THREADS/NUMBER_OF_WAITING_THREADS.
>
> ...and there are also comments above with this issue.
> expEnteringCount/expWaitingCount contain the tested patterns.
I kind of disagree.
Please, take look at the loop below:
for (int i = 0; i < NUMBER_OF_WAITING_THREADS; i++) {
expEnteringCount = isVirtual ? 0 : NUMBER_OF_ENTERING_THREADS +
i + 1;
expWaitingCount = isVirtual ? 0 : NUMBER_OF_WAITING_THREADS -
i - 1;
lockCheck.notify(); // notify waiting threads one by one
// now the notified WaitingTask has to be blocked on the
lockCheck re-enter
// entry count: 1
// count of threads waiting to enter:
NUMBER_OF_ENTERING_THREADS
// count of threads waiting to re-enter: i + 1
// count of threads waiting to be notified:
NUMBER_OF_WAITING_THREADS - i - 1
check(lockCheck, expOwnerThread(), expEntryCount(),
expEnteringCount,
expWaitingCount);
}
The comment fixed as you suggest does not look useful anymore as the tested
pattern is lost:
// entry count: expOwnerThread()
// count of threads waiting to enter: expEnteringCount
// count of threads waiting to re-enter: expEntryCount()
// count of threads waiting to be notified: expWaitingCount
check(lockCheck, expOwnerThread(), expEntryCount(),
expEnteringCount,
expWaitingCount);
}
I understand your concern but your suggestion is not that good.
We could remove these comments but the tested pattern will be thrown away with
the comments.
Would it help if we add clarifications that the comments are correct for
platform threads only?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19030#discussion_r1589041287
