On 2/11/26 05:00, Axel Boldt-Christmas wrote:
On Tue, 10 Feb 2026 17:40:30 GMT, Frederic Parain <[email protected]> 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
