On Thu, 3 Sep 2026 20:01:19 GMT, Ioi Lam <[email protected]> wrote:

>> `FieldClosure` is used for iterating fields in an object. For Valhalla, it 
>> has been enhanced to handle fields that are inside inlined fields. However, 
>> the current implementation has two problems:
>> 
>> [1] It type casts the address of an inlined field into an `oop` pointer. 
>> This is unsafe as many operations, such as getting the header of an `oop`, 
>> will not work with such an `oop` pointer:
>> 
>> https://github.com/openjdk/jdk/blob/b1f975efa481bd5e20c1b7d58a87d0866df205e6/src/hotspot/share/runtime/fieldDescriptor.cpp#L216
>> 
>> [2] The parameter `base_offset` is used in many functions. Its meaning is 
>> unclear and inconsistent.
>> 
>> https://github.com/openjdk/jdk/blob/b1f975efa481bd5e20c1b7d58a87d0866df205e6/src/hotspot/share/runtime/fieldDescriptor.cpp#L159
>> 
>> https://github.com/openjdk/jdk/blob/b1f975efa481bd5e20c1b7d58a87d0866df205e6/src/hotspot/share/oops/instanceKlass.hpp#L99-L101
>> 
>> This RFE refactors `FieldClosure` to avoid the above problems. 
>> `FieldClosure` now carries information about inlined fields. This 
>> information can be used by various field iteration code to simplify their 
>> operations. See `FieldClosure::inline_klass()` and 
>> `FieldClosure::inline_offset()`.
>> 
>> As a result, users of `FieldClosure` and `FieldDescriptor()` no longer need 
>> to perform obscure arithmetics with `InlineKlass::payload_offset()`.
>> 
>> This RFE also moves a few common operations into utility functions to avoid 
>> code duplication.
>> 
>> Also:
>> - Fixed a bug in `FlatArrayKlass::oop_print_elements_on()` in the handling 
>> of nullable elements.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Ioi Lam has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - @fparain  review comments
>  - Changed inline_xxx to flat_xxx

src/hotspot/share/cds/aotMapLogger.cpp line 879:

> 877:             for (int i = 0; i < _indent + 1; i++) _st->print("  ");
> 878:             _st->print_cr(" - [null_marker] @%d %s",
> 879:                           field_offset_in_obj + vk->null_marker_offset(),

`vk->null_marker_offset()` returns of the offset of the null-marker as an 
offset from the beginning of the object for a heap buffered value.
Shouldn't `null_marker_offset_in_payload()` be used instead?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/32565#discussion_r3960943228

Reply via email to