On Thu, 12 Feb 2026 13:52:13 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)~ (Removed in favour of
>> using a static function)
>> * 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.
>
> Axel Boldt-Christmas has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Remove mutable
src/hotspot/share/oops/inlineKlassPayload.hpp line 81:
> 79: inline address absolute_addr() const;
> 80:
> 81: InlineKlass* klass() const;
There are some inconsistency w.r.t. marking functions with `inline`:
Suggestion:
inline InlineKlass* klass() const;
src/hotspot/share/oops/inlineKlassPayload.inline.hpp line 45:
> 43:
> 44: template <typename OopOrHandle>
> 45: inline ValuePayload::StorageImpl<OopOrHandle>::StorageImpl()
There are some inconsistency w.r.t. marking the function definitions with
`inline`. I'd propose that we put the `inline` declaration in the .hpp file and
remove them from this file.
src/hotspot/share/oops/inlineKlassPayload.inline.hpp line 75:
> 73: inline ValuePayload::StorageImpl<OopOrHandle>::~StorageImpl() {
> 74: #ifdef ASSERT
> 75: if (!_absolute_addr) {
Should this be:
Suggestion:
if (!_use_absolute_addr) {
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2068#discussion_r2803190722
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2068#discussion_r2803225329
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2068#discussion_r2803177797
