On Tue, 28 May 2024 06:52:13 GMT, SendaoYan <s...@openjdk.org> wrote:
>> Hi all, >> ObjectMonitorUsage.java failed with `unexpected waiter_count` after >> [JDK-8328083](https://bugs.openjdk.org/browse/JDK-8328083) on linux x86_32. >> There are two changes in this PR: >> 1. In JvmtiEnvBase::get_object_monitor_usage function, change from >> `java_lang_VirtualThread::is_instance(thread_oop)` to >> `thread_oop->is_a(vmClasses::BaseVirtualThread_klass())`to support the >> alternative implementation. >> 2. The JvmtiEnvBase::get_object_monitor_usage does take the vthread into >> consideration when calculating nWant(mon->contentions()). >> >> >> >> Additional testing: >> - [x] linux x86_32 run all testcases in serviceability/jvmti, all testcases >> run successed expect >> `serviceability/jvmti/vthread/GetThreadState/GetThreadStateTest.java#default` >> run failed. This test also run failed before this PR, which has been >> recorded in [JDK-8333140](https://bugs.openjdk.org/browse/JDK-8333140) >> - [x] linux x86_64 run all testcases in serviceability/jvmti, all testcases >> run successed. >> >> [x64.log](https://github.com/openjdk/jdk/files/15480081/x64.log) >> [x86.log](https://github.com/openjdk/jdk/files/15480083/x86.log) > > SendaoYan has updated the pull request incrementally with one additional > commit since the last revision: > > 1. java_lang_VirtualThread::is_instance(thread_oop) -> > thread_oop->is_a(vmClasses::BaseVirtualThread_klass()); 2. calculating > nWant(mon->contentions()) The fix looks good in general but I've inlined one suggestion. src/hotspot/share/prims/jvmtiEnvBase.cpp line 1524: > 1522: nWant_Skip++; > 1523: } > 1524: } Thank you for taking care about this issue. The nWant_Skip and and the fragment with lines 1518-1524 would not be needed if the function `Threads::get_pending_threads()` is fixed instead: - if (java_lang_VirtualThread::is_instance(thread_oop)) { + if (thread_oop->is_a(vmClasses::BaseVirtualThread_klass())) { `` ------------- PR Review: https://git.openjdk.org/jdk/pull/19405#pullrequestreview-2086876665 PR Review Comment: https://git.openjdk.org/jdk/pull/19405#discussion_r1619620594
