[lworld] RFR: 8377451: [lworld] Add ValuePayload abstraction [v3]

[Frederic Parain]

On 2/11/26 05:00, Axel Boldt-Christmas wrote:
> On Tue, 10 Feb 2026 17:40:30 GMT, Frederic Parain <fparain at openjdk.org> wrote:
>
>>> Axel Boldt-Christmas has updated the pull request incrementally with one additional commit since the last revision:
>>>
>>>    Remove incorrect assert. JVM_CopyOfSpecialArray is currently broken
>> src/hotspot/share/interpreter/zero/bytecodeInterpreter.cpp line 2681:
>>
>>> 2679:         if (entry->is_flat()) {
>>> 2680:           CALL_VM(InterpreterRuntime::write_flat_field(THREAD, obj, val, entry), handle_exception);
>>> 2681:         } else {
>> The case of null-free arrays was added above, it would be nice to also add the support for null-free non-flat fields, even if they are not part of JEP 401.
> I guess `is_null_free_inline_type` is the correct property here. I did not realise that `@NullRestricted` was only applicable for fields holding value types.

Yes, in the case where the field is not flat, `is_null_free_inline_type` 
should be tested to see if a null check is required.

Valueness and nullfreeness have been tied together for a very long time 
in project Valhalla. We will eventually update the to see them at two 
distinct properties, so null-freeness could be applied to identity types 
too. But this is beyond the scope of JEP 401, so we have deferred this 
change.

There's a work in progress in the bang-world prototype to implement this 
split.


>> src/hotspot/share/oops/inlineKlassPayload.inline.hpp line 419:
>>
>>> 417: }
>>> 418:
>>> 419: inline void FlatValuePayload::copy_from_non_null(BufferedValuePayload& src) {
>> `copy_from_non_null` looks a bit weird here, because the method does something more precise: copying from a buffered value, which implies 1) the src is never null and 2) the method is allowed to update the null-marker.
>> `copy_from_buffered` looks like a better name to express what the method does.
> I did originally call it `copy_from` (buffer is implied by the argument type). But when I still used the null_reset (null payload) which is a Buffer which is representing null it was invalid if you called this method. But after removing using the null_payload it can probably be renamed back `copy_from`.
>
> Also your comment made me realise that the use in `FlatArrayKlass::copy_array` was wrong. It should not have called `copy_from_non_null` if the `copy_to` call failed. Pushed a fix and renamed the function.
>
> Will also look at removing the changes w.r.t. null reset value in this change. We should not really use it in the runtime code. Because trying to use that oop with `write` or `copy_from` is incorrect. (Might even add an assert for it, there should only ever be one such object per InlineKlass)
>
>> src/hotspot/share/oops/inlineKlassPayload.inline.hpp line 496:
>>
>>> 494:                                           InstanceKlass* klass)
>>> 495:     : FlatFieldPayload(container,
>>> 496:                        klass->field_offset(field_descriptor->index()),
>> klass->field_offset() is an expensive call, because it decodes the FieldInfoStream to get the field offset. The fieldDescriptor argument has a cheaper way to get the field offset: field_descriptor->offset().
> Right.  Sounds good. Just used the same code we already had in the `jni_[Set|Get]ObjectField`. But we already walked the field stream to find the field info, let us not walk the field stream again.
>
> -------------
>
> PR Review Comment: https://git.openjdk.org/valhalla/pull/2068#discussion_r2792012654
> PR Review Comment: https://git.openjdk.org/valhalla/pull/2068#discussion_r2791736640
> PR Review Comment: https://git.openjdk.org/valhalla/pull/2068#discussion_r2791820213