On Thu, 4 May 2023 01:53:10 GMT, Serguei Spitsyn <sspit...@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
