On Wed, 21 Feb 2024 22:34:19 GMT, Leonid Mesnik <[email protected]> wrote:
>> Serguei Spitsyn has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> review: addressed minor issue with use of []; corrected the test
>> desctiption
>
> test/hotspot/jtreg/serviceability/jvmti/GetObjectMonitorUsage/ObjectMonitorUsage.java
> line 42:
>
>> 40: * - all the above is checked for both platform and virtual threads
>> 41: * @requires vm.jvmti
>> 42: * @compile ObjectMonitorUsage.java
>
> No need to have explicit compile for the test code.
Fixed, thanks.
> test/hotspot/jtreg/serviceability/jvmti/GetObjectMonitorUsage/ObjectMonitorUsage.java
> line 204:
>
>> 202:
>> 203: // test virtual threads
>> 204: test1(false);
>
> shouldn't be true here?
Nice catch, thanks. Fixed now.
> test/hotspot/jtreg/serviceability/jvmti/GetObjectMonitorUsage/libObjectMonitorUsage.cpp
> line 105:
>
>> 103: err = jvmti->GetObjectMonitorUsage(obj, &inf);
>> 104: if (err == JVMTI_ERROR_MUST_POSSESS_CAPABILITY &&
>> !caps.can_get_monitor_info) {
>> 105: return; /* Ok, it's expected */
>
> I think we don't need this path.
Yes. Fixed, thanks.
> test/hotspot/jtreg/serviceability/jvmti/GetObjectMonitorUsage/libObjectMonitorUsage.cpp
> line 107:
>
>> 105: return; /* Ok, it's expected */
>> 106: } else if (err != JVMTI_ERROR_NONE) {
>> 107: LOG("(GetMonitorInfo#%d) unexpected error: %s (%d)\n",
>
> you could use check_jvmti_status
Fixed, thanks.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1498694748
PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1498694426
PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1498693585
PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1498693206
