On Fri, 8 Sep 2023 02:46:36 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
>> Serguei Spitsyn has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains two additional
>> commits since the last revision:
>>
>> - Merge
>> - 8312174: missing JVMTI events from vthreads parked during JVMTI attach
>
> test/hotspot/jtreg/serviceability/jvmti/vthread/VThreadEventTest/libVThreadEventTest.cpp
> line 30:
>
>> 28: #include "jvmti_common.h"
>> 29:
>> 30: #ifdef _WIN32
>
> Do we need it here?
Thanks. Simplified now.
> test/hotspot/jtreg/serviceability/jvmti/vthread/VThreadEventTest/libVThreadEventTest.cpp
> line 44:
>
>> 42:
>> 43: void JNICALL VirtualThreadEnd(jvmtiEnv *jvmti, JNIEnv* jni, jthread
>> virtual_thread) {
>> 44: std::lock_guard<std::mutex> lockGuard(lock);
>
> the atomic would be better for counters. It guarantees that counter is always
> protected.
Thanks. Alex suggested the same. Fixed now.
> test/hotspot/jtreg/serviceability/jvmti/vthread/VThreadEventTest/libVThreadEventTest.cpp
> line 62:
>
>> 60:
>> 61: void
>> 62: check_jvmti_err(jvmtiError err, const char* msg) {
>
> This function could be moved into jvmti_common.h.
Good suggestion, thanks. Fixed now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15467#discussion_r1320450305
PR Review Comment: https://git.openjdk.org/jdk/pull/15467#discussion_r1320450372
PR Review Comment: https://git.openjdk.org/jdk/pull/15467#discussion_r1320450217
