On Mon, 9 Feb 2026 10:49:38 GMT, Paul Hübner <[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. > > 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. The assert comment was wrong, ZGC does support atomic flat values as long as they do not contain oops. > 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? They can, for 401 we should only encounter atomic values, so the copy inside `write` may race, but will always atomically write valid data in the field / array element. However I do assume that the payload inside `value` is both immutable and visible to the current thread. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/2068#discussion_r2782875008 PR Review Comment: https://git.openjdk.org/valhalla/pull/2068#discussion_r2782895813
