[lworld] RFR: 8376221: [lworld] Do not store array of InlineLayoutInfo for all InstanceKlasses

[Stefan Karlsson]

On Mon, 26 Jan 2026 08:43:40 GMT, Joel Sikström <jsikstro at openjdk.org> wrote:

> Hello,
> 
> Please refer to the JBS issue for a more detailed description of the background of this change. In summary, I suggest we only keep the array of InlineLayoutInfo for InstanceKlasses which need it, which are Klasses that have fields that have been inlined.
> 
> To make the transition to this easier, I suggest we change the following properties in FieldLayoutBuilder: 
> 
> _has_inline_type_fields
> _has_flattening_information
> 
> to
> 
> _has_inlineable_fields
> _has_inlined_fields
> 
> The `_has_inlineable_fields` property is only used for printing and `_has_inlined_fields` is the property we expose out to the ClassFileParser, telling us that this class has inlined fields, so the array of InlineLayoutInfo must be "preserved" and is possible to read from. Hence, the array is now only safe to access if `InstanceKlass::has_inlined_fields` is true, or simply if the actual field being accessed is flat (`fieldDescriptor::is_flat`).
> 
> I only found one place (in ciReplay.cpp) where we access the array of InlineLayoutInfo even though we might not have any inlined fields and only fields that are inlineable. I've changed this to use the normal "reference" path for fields that aren't flat.
> 
> Testing:
> * Oracle's tier1-5, hotspot_valhalla and jdk_valhalla

I'm leaving the proper review to Valhalla-devs, but I added a few comments that could be considered.

src/hotspot/share/classfile/classFileParser.cpp line 6298:

> 6296:           if (klass != nullptr) {
> 6297:             if (klass->is_inline_klass()) {
> 6298:               set_inline_layout_info_klass(fieldinfo.index(), klass, CHECK);

I see that this can throw a Metaspace OOME (hence the CHECK) and that we later have this comment:


          // Loads triggered by the LoadableDescriptors attribute are speculative, failures must not impact loading of current class
          if (HAS_PENDING_EXCEPTION) {
            CLEAR_PENDING_EXCEPTION;
          }

Can you verify that using CHECK here is really appropriate? The `resolve_super_or_fail` uses THREAD, and I think that's what the comment refer to. But with the current patch there's a "returning" call in-between those two parts of the function.

src/hotspot/share/classfile/classFileParser.cpp line 6341:

> 6339:       _must_be_atomic, _layout_info, _inline_layout_info_array);
> 6340:   lb.build_layout();
> 6341:   _has_inlined_fields = _layout_info->_has_inlined_fields;

More of a questions to the long-time Valhalla devs: I'm a little curious why the parser has booth of these fields containing the same value?

src/hotspot/share/classfile/classFileParser.cpp line 6394:

> 6392: 
> 6393: void ClassFileParser::set_inline_layout_info_klass(int field_index, Klass* klass, TRAPS) {
> 6394:   assert(field_index >= 0 && field_index < java_fields_count(), "IOOB");

I would propose that you split && asserts into two asserts, or add an error message so that we can see what part failed if this ever asserts.

src/hotspot/share/classfile/fieldLayoutBuilder.cpp line 856:

> 854:         group->add_oop_field(idx);
> 855:       } else {
> 856:         assert(_inline_layout_info_array != nullptr && _inline_layout_info_array->adr_at(field_index)->klass() != nullptr, "Array must have been set up");

&& assert - see previous comment

src/hotspot/share/classfile/fieldLayoutBuilder.cpp line 930:

> 928:       bool use_atomic_flat = _must_be_atomic; // flatten atomic fields only if the container is itself atomic
> 929:       LayoutKind lk = field_layout_selection(fieldinfo, _inline_layout_info_array, use_atomic_flat);
> 930:       const int field_index = (int)fieldinfo.index();

Are `idx` and `field_index` different here?

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

PR Review: https://git.openjdk.org/valhalla/pull/1966#pullrequestreview-3705382128
PR Review Comment: https://git.openjdk.org/valhalla/pull/1966#discussion_r2727022060
PR Review Comment: https://git.openjdk.org/valhalla/pull/1966#discussion_r2727061299
PR Review Comment: https://git.openjdk.org/valhalla/pull/1966#discussion_r2727067158
PR Review Comment: https://git.openjdk.org/valhalla/pull/1966#discussion_r2727080548
PR Review Comment: https://git.openjdk.org/valhalla/pull/1966#discussion_r2727092250