On Wed, 9 Sep 2026 19:09:14 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: > > - clean up: ValuePayloadContext::_klass is used only in asserts > - Alternative
> I think that there's a relative easy change that can be made to keep > FieldClosure "clean", and I have a version of that here: > > Diff: > [stefank/jdk@pull/32565...stefank:jdk:alt_32565](https://github.com/stefank/jdk/compare/pull/32565...stefank:jdk:alt_32565) > > Branch: > [master...stefank:jdk:alt_32565](https://github.com/openjdk/jdk/compare/master...stefank:jdk:alt_32565) > > The patch moves the fields added to FieldClosure out to its own struct, that > I call `ValuePayloadContext`. Whenever the code is descending into a flat > field/element, it sets up a new `ValuePayloadContext` to describe the current > value payload that is being iterated. Hi Stefan, this does look a lot cleaner. I've integrated your patch into this PR. Also, as you fixed the comment about `this->field_holder()` in `fieldDescriptor::field_offset_in_obj()`, I realized that the only place we use `vpc->klass()` outside of asserts is in `field_offset_in_obj()`, but we already know what this class should be. So I put `vpc->klass()` inside `DEBUG_ONLY`. I think it's still worth having `vpc->klass()` to make the code more readable. In the future, if `ValuePayloadContext` is used in other places without a `fieldDescriptor`, we can remove the `DEBUG_ONLY` macros. BTW, I removed your comment `(in other cases this could be an abstract value class)`. As far as I know, a flattened field cannot be of an abstract value type. E.g., this field will not be flattened. @NullRestricted Number n; ------------- PR Comment: https://git.openjdk.org/jdk/pull/32565#issuecomment-5607441399
