RFR: 8328083: degrade virtual thread support for GetObjectMonitorUsage [v3]

[Serguei Spitsyn]

On Thu, 2 May 2024 21:47:50 GMT, Chris Plummer <cjplummer at 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