RFR: 8299414: JVMTI FollowReferences should support references from VirtualThread stack [v14]

[Alex Menkov]

On Fri, 5 May 2023 05:59:49 GMT, Serguei Spitsyn <sspitsyn at 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