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

Joel Sikström jsikstro at openjdk.org
Mon Jan 26 11:53:41 UTC 2026


On Mon, 26 Jan 2026 10:30:57 GMT, Stefan Karlsson <stefank 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
>
> 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?

I don't think they ever are. I added an assert and ran tier1-3, which never hit. Maybe @fparain can shed some light on this? Otherwise I think we should use `field_info.index()` in favor of `idx` and maybe replace the GrowableArrayIterator with a range-based for loop, like this:

for (FieldInfo fi : *_field_info) {
  ...
}

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1966#discussion_r2727311083


More information about the valhalla-dev mailing list