[lworld] RFR: 8373864: [lworld] Hide and rename InstanceKlassFixedBlock [v2]
Frederic Parain
fparain at openjdk.org
Wed Dec 17 13:32:29 UTC 2025
On Wed, 17 Dec 2025 13:07:44 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:
>> Sub-classes of `InstanceKlass` can't have C++ fields because they would end up overlayed on top of the vtable and other dynamically sized sections of the `InstanceKlass` object.
>>
>> To handle that the `InlineKlass` has a companion class named `InlineKlassFixedBlock`, which lists all the member fields that belongs to the InlineKlass, and an instance of that gets stamped into the `InlineKlass` object after the parts that are provided by the `InstanceKlass`.
>>
>> I propose a few changes:
>>
>> 1) Move `InlineKlassFixedBlock` away from instanceKlass.hpp and place it inside inlineKlass.hpp instead.
>>
>> 2) Nest `InlineKlassFixedBlock` it inside `InlineKlass`. It's only `InlineKlass `(and the compilers) that touch these fields, so it doesn't have to be a public, top-level class.
>>
>> 3) Rename it from `InlineKlassFixedBlock` to `InlineKlass::Members`. I think that "fixed block" term is unclear and doesn't help the reader understand its role in the `InlineKlass`. Hopefully, the name `Members` is a clearer.
>>
>> WDYT?
>
> 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
Marked as reviewed by fparain (Committer).
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!
-------------
PR Review: https://git.openjdk.org/valhalla/pull/1812#pullrequestreview-3587762946
PR Review Comment: https://git.openjdk.org/valhalla/pull/1812#discussion_r2627080706
More information about the valhalla-dev
mailing list