On Wed, 3 May 2023 22:02:30 GMT, Alex Menkov <[email protected]> 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
