On Tue, 14 May 2024 23:13:28 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:

>> I don't understand the issue with the updated commented. It is precisely 
>> telling you what the expected "count" values should be. If you leave the 
>> macros in the comment, then the comment is wrong for virtual threads. If you 
>> want to keep the macros in the comment, you need to add something like "... 
>> or 0 for virtual threads".
>> 
>> BTW, the "re-enter" comment should continue to be "i + 1". I'm not sure why 
>> it was changed to "expEntryCount()".
>
> Okay, please, let me explain this one more time.
> The original comments before method `check()` calls describe the testing 
> scenario but not the numbers expected to be returned by the JVMTI 
> `GetObjectMonitorUsage`.
> For instance, if the testing scenario says: "count of threads waiting to 
> enter: NUMBER_OF_ENTERING_THREADS" then it means there is a real number of 
> these threads waiting to enter the monitor. And it does not matter if they 
> are platform or virtual threads. They are really waiting to enter the 
> monitor. However, the JVMTI `GetObjectMonitorUsage` won't include virtual 
> threads into the returned results.
> 
> Now, I'm suggesting to add the following header for comments before each 
> `check()` method call:
> 
> +            // The numbers below describe the testing scenario, not the 
> expected results.
> +            // The expected numbers are different for virtual threads because
> +            // they are not supported by JVMTI GetObjectMonitorUsage.
> 
> Would it work for you?

> BTW, the "re-enter" comment should continue to be "i + 1".
> I'm not sure why it was changed to "expEntryCount()".

It depends on what are we trying to describe. We either describe the testing 
scenario (the number of threads doing something) or the expected results. I 
understood that you wanted to describe the results instead of the scenario. And 
then it becomes problematic to do so as you can see.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19030#discussion_r1600772051

Reply via email to