RFR: 8299414: JVMTI FollowReferences should support references from VirtualThread stack

[Chris Plummer]

On Thu, 30 Mar 2023 22:58:12 GMT, Alex Menkov <amenkov at openjdk.org> wrote:

> The fix updates JVMTI FollowReferences implementation to report references from virtual threads:
> - added heap scanning to report unmounted vthreads;
> - stacks of mounted vthreads are splitted into 2 parts (vittual thread stack and carrier thread stack), references are reported with correct thread id/class and object tags/frame depth;
> - common code to handle stack frames are moved into separate class;



test/hotspot/jtreg/serviceability/jvmti/vthread/FollowReferences/VThreadStackRefTest.java line 54:

> 52:                 }
> 53:             }
> 54:             await(dumpedLatch);

await() seems unnecessary given the use the !timeToStop flag.

test/hotspot/jtreg/serviceability/jvmti/vthread/FollowReferences/VThreadStackRefTest.java line 83:

> 81:             System.out.println(referenced.getClass());
> 82:         });
> 83:         vthreadEnded.join();

Add comment that says something like "Make sure this vthread has exited so we can test that it no longer holds any stack references".

test/hotspot/jtreg/serviceability/jvmti/vthread/FollowReferences/VThreadStackRefTest.java line 85:

> 83:         vthreadEnded.join();
> 84: 
> 85:         Thread.sleep(2000); // wait for reference and unmount

I think what you mean is you need to wait until the threads have made enough progress to create the references, and then you need to wait until they have had a chance to amount due to the await() call. This should be made more clear in the comments.

BTW, you could choose to get JVMTI VIRTUAL_THREAD_UNMOUNT events, and instead block here until you get them all, but doing a sleep is a lot easier.

test/hotspot/jtreg/serviceability/jvmti/vthread/FollowReferences/VThreadStackRefTest.java line 94:

> 92:             // expected to be unreported as stack local
> 93:             new TestCase(VThreadUnmountedEnded.class, 0, 0)
> 94:         };

I think it would be useful the have a test case which has expected_cnt > 1.

test/hotspot/jtreg/serviceability/jvmti/vthread/FollowReferences/libVThreadStackRefTest.cpp line 72:

> 70:             jvmtiHeapReferenceInfoStackLocal *stackInfo = (jvmtiHeapReferenceInfoStackLocal *)reference_info;
> 71:             refCounters.count[index]++;
> 72:             refCounters.threadId[index] = stackInfo->thread_id;

If `count` is >1 at this point, can this line be an assert? I assume the threadId should never change for any given index once it is set.

-------------

PR Review: https://git.openjdk.org/jdk/pull/13254#pullrequestreview-1369892549
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1156534139
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1156534779
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1156538712
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1156540510
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1156520354