On Wed, 17 Dec 2025 13:07:44 GMT, Stefan Karlsson <[email protected]> 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
