On Thu, 10 Sep 2026 16:55:32 GMT, Ioi Lam <[email protected]> wrote:

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

I guess this all depends on how you like to think about the code. Right now the 
code has been tasked with understanding that it is being printed as a field 
inside a flattened field of an object. With my suggestion that gets abstracted 
away, and reading the code you only have to care that some code didn't like the 
standard way of fetching the offset, and that this function doesn't have to 
deal with those details. 

I'm fine with leaving this as-is.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/32565#discussion_r3982599759

Reply via email to