On Mon, 9 Feb 2026 09:35:48 GMT, Axel Boldt-Christmas <[email protected]>
wrote:
> Create a wrapper class which represents the payload of an InlineKlass object.
>
> The current convention is to use a `void*` representing where the payload
> starts, the `InlineKlass*` (as we do not always have a header when flattened)
> and a `LayoutKind` (describing the payloads layout).
>
> I suggest we introduce something like a `ValuePayload` which encapsulate
> these properties. As well as a hierarchy built upon these, with the proper
> interfaces implemented.
> * ValuePayload (Any payload)
> * RawValuePayload (Payload with holder erased)
> * BufferedValuePayload (Payload of normal heap object)
> * FlatValuePayload (Payload of flattened value)
> * FlatFieldPayload (Payload of flattened field)
> * FlatArrayPayload (Payload of flattened array element)
>
> The goal is to both make interfaces clearer, and easier to understand. As
> well as consolidating the implementation in one place rather than spread
> across different subsystems.
>
> Each type (except RawValuePayload) also allows for the creation of a Handle,
> (thread local, or in an OopStorage) for keeping the payload as a thread or
> global root.
>
> The ValuePayload class is also the interface for interacting with the Access
> API for InlineKlass objects.
>
> * Testing
> * Running tier 1-4 with preview enabled
> * Running app tests with preview enabled
> * Running normal tier 1-5
>
> #### _Extra Notes:_
> * The `OopHandle` type is there so that we can migrate the JVMTI payload
> abstraction implementation to using this instead. (Future RFE)
> * Some interfaces got cleaned up. Some are unused. Like the `null_payload`
> which was superseded by the `Access::value_store_null`. C1 still uses the
> `.null_reset` but if that dependency is removed we should be able to remove
> that weird object all together.
> * Simply adding the Java to VM transition deep inside the payload code
> created a circular include dependency here. So rather than fixing that, I
> implemented the relevant bytecodes in the BytecodeInterpreter.
Thanks for this work, it really improves this code a lot. I've left some very
superficial comments, I plan on doing a more in-depth review pass down the line.
src/hotspot/share/gc/z/zBarrierSet.inline.hpp line 536:
> 534: const LayoutKind lk = dst.get_layout_kind();
> 535: assert(!LayoutKindHelper::is_null_free_flat(lk),
> 536: "ZGC cannot handle atomic flat values");
Should this be null free instead? `LKH::is_null_free_flat` can be true for
nonatomic values.
src/hotspot/share/interpreter/zero/bytecodeInterpreter.cpp line 1505:
> 1503: CASE(_aaload): {
> 1504: ARRAY_INTRO(-2);
> 1505: if (((objArrayOop) arrObj)->is_flatArray()) {
As you said it'd be nice to clean this up at some point.
src/hotspot/share/prims/unsafe.cpp line 468:
> 466: LayoutKind lk = (LayoutKind)layoutKind;
> 467: FlatValuePayload payload =
> FlatValuePayload::construct_from_parts(base, vk, offset, lk);
> 468: payload.write(inlineOop(JNIHandles::resolve(value)), CHECK);
Maybe a stupid question but is it safe to write the payload? Could another
thread be writing to it at the same time?
-------------
PR Review:
https://git.openjdk.org/valhalla/pull/2068#pullrequestreview-3772582927
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2068#discussion_r2781937778
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2068#discussion_r2781949920
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2068#discussion_r2782568877
