On Thu, 10 Sep 2026 10:20:28 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 one additional commit
> since the last revision:
>
> Restored ValuePayloadContext::klass() and added test case with abstract
> value class
src/hotspot/share/runtime/fieldDescriptor.hpp line 172:
> 170: _klass(klass), _offset_in_obj(offset_in_obj) {
> 171: precond(klass != nullptr);
> 172: precond(offset_in_obj > 0);
Putting the initialization list on the same indentation makes the flow harder
to read. In other parts of the JVM we take extra care to not do this by doing
either of:
ValuePayloadContext(ValueKlass* klass, int offset_in_obj) :
_klass(klass), _offset_in_obj(offset_in_obj) {
precond(klass != nullptr);
precond(offset_in_obj > 0);
or
ValuePayloadContext(ValueKlass* klass, int offset_in_obj)
: _klass(klass), _offset_in_obj(offset_in_obj) {
precond(klass != nullptr);
precond(offset_in_obj > 0);
or
ValuePayloadContext(ValueKlass* klass, int offset_in_obj) :
_klass(klass), _offset_in_obj(offset_in_obj)
{
precond(klass != nullptr);
precond(offset_in_obj > 0);
Maybe use one of these here?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32565#discussion_r3978423168
