[lworld] RFR: 8377451: [lworld] Add ValuePayload abstraction
Axel Boldt-Christmas
aboldtch at openjdk.org
Mon Feb 9 22:05:17 UTC 2026
On Mon, 9 Feb 2026 13:51:58 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:
>> src/hotspot/share/oops/inlineKlassPayload.inline.hpp line 222:
>>
>>> 220: inline address ValuePayload::get_address() const {
>>> 221: return reinterpret_cast<address>(cast_from_oop<intptr_t>(get_holder()) +
>>> 222: _storage._offset);
>>
>> Could this be just:
>> Suggestion:
>>
>> return cast_from_oop<address>(get_holder()) + _storage._offset;
>
> In the `RawValuePayload` case the holder is `nullptr`, we would get `nullptr` + non-zero offset UB.
> Probably worth a comment.
Which I know, the offset is not an offset it is an address there. Maybe we should rename it. The holder + offset, should probably just be a tagged union. And have the is_raw property just be the tag.
I will look at doing this.
>> src/hotspot/share/oops/inlineKlassPayload.inline.hpp line 289:
>>
>>> 287: // non null. It is the callers responsibility to ensure that this is a
>>> 288: // valid non null value.
>>> 289: src.mark_as_non_null();
>>
>> Is it guaranteed that only one thread is accessing the `src` here? Maybe more of a question to @fparain
>
> Probably good to double check. I do not think we assume the invariant that this is not a concurrently accessed object.
>
> My assumption is that `BufferedValuePayload` have an immutable payload, only the null marker may be indeterminate. A buffered payload cannot be transformed from a valid object -> null.
>
> With the removal of the private buffer, (which now instead uses a FlatArray of length 1) we should not have mutable buffered objects out in Java. Only mutable flat fields and flat array elements. But they do not have the immutability assumption.
And I am not convinced that we have the correct synch primitives everywhere for the immutable invariant to hold. But I think that the the immutable invariant is the intended design.
If it was not and we are allowed to observe the pre-copy / pre-construction contents of a buffered object I think both this and the previous implementation is broken w.r.t. concurrent reads.
-------------
PR Review Comment: https://git.openjdk.org/valhalla/pull/2068#discussion_r2782775162
PR Review Comment: https://git.openjdk.org/valhalla/pull/2068#discussion_r2782847142
More information about the valhalla-dev
mailing list