On Fri, 5 May 2023 05:59:49 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> Alex Menkov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Updated test
>
> test/hotspot/jtreg/serviceability/jvmti/vthread/FollowReferences/libVThreadStackRefTest.cpp
> line 39:
>
>> 37: jint testClassCount;
>> 38: jint *count;
>> 39: jlong *threadId;
>
> Camel case is the Java naming convention for identifiers.
> Tests normally use camel case only for native methods which are called from
> Java.
fixed
> test/hotspot/jtreg/serviceability/jvmti/vthread/FollowReferences/libVThreadStackRefTest.cpp
> line 106:
>
>> 104: extern "C" JNIEXPORT jint JNICALL
>> 105: Agent_OnLoad(JavaVM *vm, char *options, void *reserved) {
>> 106: if (vm->GetEnv(reinterpret_cast<void **>(&jvmti), JVMTI_VERSION) !=
>> JNI_OK || jvmti == nullptr) {
>
> Nit: This line is long and non readable. There are many examples in tests how
> it is normally done.
done
> test/hotspot/jtreg/serviceability/jvmti/vthread/FollowReferences/libVThreadStackRefTest.cpp
> line 113:
>
>> 111: memset(&capabilities, 0, sizeof(capabilities));
>> 112: capabilities.can_tag_objects = 1;
>> 113: //capabilities.can_support_virtual_threads = 1;
>
> The line 113 can be removed now.
done
> test/hotspot/jtreg/serviceability/jvmti/vthread/FollowReferences/libVThreadStackRefTest.cpp
> line 130:
>
>> 128: Java_VThreadStackRefTest_test(JNIEnv* env, jclass clazz, jobjectArray
>> classes) {
>> 129: jsize classesCount = env->GetArrayLength(classes);
>> 130: for (int i=0; i<classesCount; i++) {
>
> Spaces are missed arounf '=' and '<' signs.
fixed
> test/hotspot/jtreg/serviceability/jvmti/vthread/FollowReferences/libVThreadStackRefTest.cpp
> line 154:
>
>> 152: }
>> 153:
>> 154: static void printtCreatedClass(JNIEnv* env, jclass cls) {
>
> Why is printt with 'tt' ?
ttypo :) fixed
> test/hotspot/jtreg/serviceability/jvmti/vthread/FollowReferences/libVThreadStackRefTest.cpp
> line 167:
>
>> 165:
>> 166: extern "C" JNIEXPORT void JNICALL
>> 167: Java_VThreadStackRefTest_createObjAndCallback(JNIEnv* env, jclass
>> clazz, jclass cls, jobject callback) {
>
> Some comment would be helpful about what this function does.
added
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1186547290
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1186547055
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1186547020
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1186547091
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1186547193
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1186547244
