On Thu, 10 Sep 2026 11:15:58 GMT, Stefan Karlsson <[email protected]> wrote:
>> Ioi Lam has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> fixed whitespaces
>
> src/hotspot/share/cds/heapShared.hpp line 368:
>
>> 366:
>> 367: class OopFieldPusher;
>> 368: class FlatFieldKlassFinder;
>
> Shouldn't this be removed?
> Suggestion:
This is an inner class. See `HeapShared:: FlatFieldKlassFinder` in
heapShared.cpp.
> src/hotspot/share/oops/flatArrayKlass.cpp line 436:
>
>> 434: } else {
>> 435: ValuePayloadContext vpc{vk, fa->value_offset_as_int(index,
>> layout_helper())};
>> 436: FieldPrinter print_field(st, fa, /*indent*/0, &vpc);
>
> I guess this is a preference, but maybe let everything breath a little ...
> Suggestion:
>
> FieldPrinter print_field(st, fa, /* indent */ 0, &vpc);
Fixed. I also changed the indent to 1 to make the output similar to
aotMapLogger:
{0x000000071800fd00} - klass: 'Point'[] - flags: is_cloneable_fast
- length: 3
- Index 0 offset 16:
- final value flat 'x' (fields 0x00000003) 'Ljava/lang/Integer;' @16
LayoutKind: NULL_FREE_NON_ATOMIC_FLAT Flat value null-free type field
'java/lang/Integer':
- private final value 'value' (fields 0x00000000) 'I' @16 1111
(0x00000457)
> src/hotspot/share/runtime/fieldDescriptor.cpp line 174:
>
>> 172:
>> 173: // Print information (such as type, name, offset) of this field.
>> 174: void fieldDescriptor::print_on(outputStream* st, const
>> ValuePayloadContext* vpc) const {
>
> FWIW, I had a version of this that skipped passing down a vpc. Instead it
> sent in an optional "offset_override", which brought down the
> `this->field_offset_in_obj(vpc);` calculation from `print_on_for`. This
> allows you to make `print_on` vpc agnostic. Just food for thought.
With an int field of some sort of offset, it hard to understand what it
represents, and you have to consult the caller. With a ValuePayloadContext
parameter, the meaning is more obvious.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32565#discussion_r3981532007
PR Review Comment: https://git.openjdk.org/jdk/pull/32565#discussion_r3981527072
PR Review Comment: https://git.openjdk.org/jdk/pull/32565#discussion_r3981514644
