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

Alex Menkov amenkov at openjdk.org
Thu May 4 20:55:37 UTC 2023


On Thu, 4 May 2023 01:53:10 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> 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`

done

> 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`.

changed variable names.
I think "offset" is not good name here, it's unclear what the offset is.
slot_offset shows that the offset is for reported slot parameter

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1185498169
PR Review Comment: https://git.openjdk.org/jdk/pull/13254#discussion_r1185499481


More information about the hotspot-dev mailing list