On Wed, 17 Dec 2025 10:21:24 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?
Looks good overall, just minor comments.
src/hotspot/share/ci/ciMethod.hpp line 44:
> 42: class BCEscapeAnalyzer;
> 43: class InlineTree;
> 44: class SigEntry;
Why do we need this declaration? AFAICT `ciMethod`'s method should be
unaffected by this change?
src/hotspot/share/oops/inlineKlass.hpp line 90:
> 88:
> 89: Members& members() {
> 90: assert(_adr_inline_klass_members != 0, "Should have been
> initialized");
Nitpick: `nullptr` for clarity as that's what we initialize it as?
-------------
Marked as reviewed by phubner (Committer).
PR Review:
https://git.openjdk.org/valhalla/pull/1812#pullrequestreview-3587240158
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1812#discussion_r2626646022
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1812#discussion_r2626660311
