On Wed, 28 Jan 2026 12:30:09 GMT, Paul Hübner <[email protected]> wrote:
>> Joel Sikström has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Exception check should really be an assert >> - Move inlineable check to static helper > > 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... > 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() ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/1966#discussion_r2737999879 PR Review Comment: https://git.openjdk.org/valhalla/pull/1966#discussion_r2738000437
