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

[Serguei Spitsyn]

On Wed, 10 May 2023 23:41:07 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:
> 
>   some refactoring
>   
>   added StackRefCollector::process_frames;
>   used single RegisterMap instance;
>   used RegisterMap::WalkContinuation::include for RegisterMap;

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

> 2256: 
> 2257:   bool set_thread(oop o);
> 2258:   // sets the thread and reports the reference to it with the specified kind.

Could I ask you to polish comments a little bit?
Let's use the following Consistent Comment Rule (CCR):
If comment is started with a capitol letter then it should be ended with dot.
Otherwise, it should not be ended with dot.
I'll mark with CCR other comments that are inconsistent with CCR.

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

> 2260: 
> 2261:   bool do_frame(vframe* vf);
> 2262:   // handles frames until vf->sender() is null.

CCR

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

> 2288:         continue;
> 2289:       }
> 2290: 

Unneeded empty line.

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

> 2310:   } else {
> 2311:     if (_last_entry_frame != nullptr) {
> 2312:       // JNI locals for the entry frame

CCR

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

> 2326:     javaVFrame* jvf = javaVFrame::cast(vf);
> 2327: 
> 2328:     // the jmethodID

It is unlikely this comment helps.

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

> 2339:       }
> 2340: 
> 2341:       // Follow oops from compiled nmethod

CCR

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

> 2795: // Reports the thread as JVMTI_HEAP_REFERENCE_THREAD,
> 2796: // walks the stack of the thread, finds all references (locals
> 2797: // and JNI calls) and reports these as stack references

CCR

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

> 2827:                       RegisterMap::WalkContinuation::include);
> 2828: 
> 2829:   // first handle mounted vthread (if any).

CCR

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

> 2831:     frame f = java_thread->last_frame();
> 2832:     vframe* vf = vframe::new_vframe(&f, &reg_map, java_thread);
> 2833:     // report virtual thread as JVMTI_HEAP_REFERENCE_OTHER.

CCR

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

> 2836:     }
> 2837:     // split virtual thread and carrier thread stacks by vthread entry ("enterSpecial") frame,
> 2838:     // consider vthread entry frame as the last vthread stack frame.

CCR

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

> 2899:   StackRefCollector stack_collector(tag_map(), &blk, nullptr);
> 2900:   // reference to the vthread is already reported.
> 2901:   if (!stack_collector.set_thread(vt)) {

CCR

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1190539577
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1190539725
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1190539996
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1190540181
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1190540801
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1190540873
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1190541100
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1190541281
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1190541349
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1190541442
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1190541874