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

Serguei Spitsyn sspitsyn at openjdk.org
Thu May 4 01:58:20 UTC 2023


On Wed, 3 May 2023 22:02:30 GMT, Alex Menkov <amenkov at openjdk.org> wrote:

>> The fix updates JVMTI FollowReferences implementation to report references from virtual threads:
>> - unmounted vthreads are detected, their stack references for JVMTI_HEAP_REFERENCE_STACK_LOCAL/JVMTI_HEAP_REFERENCE_JNI_LOCAL;
>> - stacks of mounted vthreads are splitted into 2 parts (virtual thread stack and carrier thread stack), references are reported with correct thread id/class tag/object tags/frame depth;
>> - common code to handle stack frames are moved into separate class;
>> 
>> Threads are reported as:
>> - platform threads: JVMTI_HEAP_REFERENCE_THREAD (as before);
>> - mounted vthreads (synthetic references, consider them as heap roots because carrier threads are roots): JVMTI_HEAP_REFERENCE_OTHER;
>> - unmounted vthreads: not reported as heap roots.
>
> Alex Menkov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   feedback

src/hotspot/share/prims/jvmtiTagMap.cpp line 2231:

> 2229: 
> 2230: // Helper class to collect/report stack roots.
> 2231: class StackRootCollector {

We discussed privately about the following renamings:
 - `StackRootCollector` => `StackRefCollector`
 - `collect_stack_roots` => `collect_stack_refs`
 - `collect_vthread_stack_roots` => `collect_vthread_stack_refs`

src/hotspot/share/prims/jvmtiTagMap.cpp line 2284:

> 2282:   for (int index = 0; index < values->size(); index++) {
> 2283:     if (values->at(index)->type() == T_OBJECT) {
> 2284:       oop o = values->obj_at(index)();

I'd suggest to get rid of one-letter identifier like `o` and `c`.
They variables can be renamed to `obj` and `cont` instead.
It'd better to rename `slot_offset` to `offset`.

src/hotspot/share/prims/jvmtiTagMap.cpp line 2893:

> 2891:   HandleMark hm(current_thread);
> 2892: 
> 2893:   StackChunkFrameStream<ChunkFrames::Mixed> fs(chunk);

There are ways to avoid using the `StackChunkFrameStream`.
You can find good examples in the jvmtiEnvBase.cpp.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1184469330
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1184466352
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1184470111


More information about the serviceability-dev mailing list