On Mon, 27 Oct 2025 18:47:17 GMT, Coleen Phillimore <[email protected]> wrote:
>> Frederic Parain has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add comment
>
> src/hotspot/share/classfile/fieldLayoutBuilder.cpp line 1301:
>
>> 1299: }
>> 1300: last_idx = last_idx == -1 ? 0 : last_idx;
>> 1301: int start = map->adr_at(last_idx)->first > offset ? 0 : last_idx;
>
> last_idx is an optimization to try to prevent searching through the field map
> to find where to insert {offset,size} for the new entry. When would it not
> be ascending? super classes and inlined classes are inserted in order that
> they're found in the LayoutBlocks so wouldn't the offsets be ascending?
You answered this in slack for me. Can you put this at the top of this
function:
The insert_segment creates an ordered list of maps {offset, size} from the
LayoutRawBlocks. LayoutRawBlocks are queued in ascending offset order, and are
processed in this order. This allows the last_idx optimization which would be
critical for classes with a huge number of fields.
But, this property is not required, and, if you look at the entire set of
fields being processed, they might not all be processed by ascending offset
order.
Inherited fields are inserted first, by copying them from the super-class, and
then local fields are processed. It is possible to have local fields
interleaved with super-class fields. So all fields are not necessarily
processed in ascending offset order. The algorithm tries to do it, and
optimizes the insertion when it is done that way, but it also handles cases
where this is not the case.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1695#discussion_r2466757559
