On Thu, 23 Oct 2025 20:52:24 GMT, Frederic Parain <[email protected]> wrote:
>> src/hotspot/share/classfile/fieldLayoutBuilder.cpp line 1361:
>>
>>> 1359: _nonoop_acmp_map = new GrowableArray<Pair<int,int>>();
>>> 1360: _oop_acmp_map = new GrowableArray<int>();
>>> 1361: if (_is_empty_inline_class) return;
>>
>> Why not return first before allocating these GrowableArray in resource area?
>
> It streamlines the code in InstanceKlass::fill_instance_klass() by not having
> to handle the null map case.
Okay. I can't decide if you need a comment to prevent someone from changing
this to have the quick exit. I guess they'll get a crash for the null case.
>> src/hotspot/share/classfile/fieldLayoutBuilder.cpp line 1381:
>>
>>> 1379: case LayoutRawBlock::REGULAR:
>>> 1380: {
>>> 1381: FieldInfo* fi = _field_info->adr_at(b->field_index());
>>
>> Is there an assumption that the block field indexes are in ascending order?
>
> I'm not sure I understand the question.
> LayoutRawBlocks represent the fields layout, they are chained in ascending
> offset order.
> The field_index is the index in the FieldInfo array (from the class file)
> where this field is declared.
Yes, that was the question. I meant to ask if the offsets are in ascending
order, so that makes the code to coalesce them simpler. Can you add a comment
at 1367 that the field offsets are in ascending order and that makes reading
this easier.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1695#discussion_r2461400686
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1695#discussion_r2461413743
