On Wed, 28 Jan 2026 18:40:16 GMT, Joel Sikström <[email protected]> wrote:

>> src/hotspot/share/classfile/fieldLayoutBuilder.cpp line 108:
>> 
>>> 106:   }
>>> 107: 
>>> 108:   if (!fieldinfo.field_flags().is_injected() &&
>> 
>> What's the rationale of not inlining injected fields? Is there a reason we 
>> need to treat them specially? I think I see this behaviour in the removed 
>> code as well, but I'm curious why this is the case.
>
> The logic is the same as we had before, I just moved it to this helper. With 
> that said, I'm not sure why we check this, as an injected field should likely 
> not be present in `_inline_layout_info_array` so should not get through this 
> check at all...

In that case, can we convert the injected check to an assertion? And perhaps 
see what happens?

>> src/hotspot/share/classfile/fieldLayoutBuilder.cpp line 965:
>> 
>>> 963:         assert(_inline_layout_info_array != nullptr, "Array must have 
>>> been created");
>>> 964:         assert(_inline_layout_info_array->adr_at(field_index)->klass() 
>>> != nullptr, "Klass must have been set");
>>> 965:         _has_inlined_fields = true;
>> 
>> Nit: can we have some sort of `set/mark_inlined_fields_checked` which does 
>> this? I saw the exact same code block above.
>
> Maybe we can hold off on this and address that in a follow-up? I think it 
> would be good to try to extract common logic from the following two methods, 
> which are very similar right now:
> 
> FieldLayoutBuilder::inline_class_field_sorting()
> FieldLayoutBuilder::regular_field_sorting()

Works for me, thanks!

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

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

Reply via email to