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

[Axel Boldt-Christmas]

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

Looks good. Just one comment on using a more precis type signature. 

A pre-existing issue which I wonder if we can do better. And a general question of how the failure paths are suppose to work for these new pre-loaded classes.

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 think this is a property already precent in the code but this change made me reflect on this. 

What are the expected interactions for pre-loaded classes if we fail to load parse the class file which contains them. For example here if we have just resolved the pre-loaded class, and then get metaspace out of memory and stop loading this class file. 

Is this the correct (specification wise) interaction for partially failed class file parsing?

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

> 6391: }
> 6392: 
> 6393: void ClassFileParser::set_inline_layout_info_klass(int field_index, Klass* klass, TRAPS) {

This should be typed as `InstanceKlass* klass`. 

2 out 3 call sites have that type, and the remaining can be trivially changed to `InstanceKlass*`
https://github.com/jsikstro/valhalla/blob/ae50201a1b19eaa7a53bce222dfc93ab61dd31f9/src/hotspot/share/classfile/classFileParser.cpp#L6293

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

> 846:           || (!fieldinfo.field_flags().is_injected()
> 847:               && _inline_layout_info_array != nullptr && _inline_layout_info_array->adr_at(field_index)->klass() != nullptr
> 848:               && !_inline_layout_info_array->adr_at(field_index)->klass()->is_identity_class())) {

Pre-existing:
I wonder if this should be a helper function. (Maybe the whole predicate should be)

But we are really asking is the klass of field type at `field_index` not an identity class. The null checks seems like implementation details.

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

PR Review: https://git.openjdk.org/valhalla/pull/1966#pullrequestreview-3705431560
PR Review Comment: https://git.openjdk.org/valhalla/pull/1966#discussion_r2727103461
PR Review Comment: https://git.openjdk.org/valhalla/pull/1966#discussion_r2727067959
PR Review Comment: https://git.openjdk.org/valhalla/pull/1966#discussion_r2727087888