On Wed, 17 Dec 2025 13:29:54 GMT, Frederic Parain <[email protected]> wrote:

>> Stefan Karlsson has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains three additional 
>> commits since the last revision:
>> 
>>  - Fix null check
>>  - Merge remote-tracking branch 'valhalla/lworld' into 
>> lworld_8373864_inline_klass_members
>>  - 8373864: [lworld] Hide and rename InstanceKlassFixedBlock
>
> src/hotspot/share/oops/inlineKlass.hpp line 53:
> 
>> 51:   // features (see InstanceKlass::size). Therefore, we can't put C++ 
>> fields
>> 52:   // directly into the InlineKlass class, but instead we stamp out a 
>> block of
>> 53:   // these members after the part of the object that comes from the 
>> InstanceKlass.
> 
> Suggestion: clarify in the comment that the goal was to have the same offset 
> for the vtable in both InstanceKlass and InlineKlass, so the vtable could be 
> accessed without having to differentiate between the two kinds of Klass.
> 
> Otherwise, nice rework!

Thanks! I've updated the comment to go into more details. Please, take a look 
and see if this is better.

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1812#discussion_r2627201378

Reply via email to