On Tue, 24 Mar 2026 02:24:23 GMT, David Holmes <[email protected]> wrote:
> Simple cleanup:
> - we don't need to discern between FlatArrayKlass and RefArrayKlass so can
> revert to the same code as present in mainline
> - we add an assert to check we have a refined-array type
>
> Testing
> - all hotspot JNI tests that use SetObjectArrayElement
> - tiers 1-3 sanity
>
> Thanks
Looks good to me. Some thoughts that popped up when reading this change:
src/hotspot/share/prims/jni.cpp line 2395:
> 2393: assert(a->klass()->is_refined_objArray_klass(), "must be");
> 2394: if (v == nullptr ||
> v->is_a(ObjArrayKlass::cast(a->klass())->element_klass())) {
> 2395: a->obj_at_put(index, v, THREAD);
Some musing, but not entirely necessary for the review: I'm not entirely sure
that changing `CHECK` to `THREAD` is beneficial for readers of the code. With
`CHECK` I understood that the code returned at that point if an exception is
thrown. With `THREAD` I know have to read to the end of the function to see
that there's no other code that is being run when we hit an exception. OTOH,
that's probably a prudent thing to do for both cases given how this code is
if/else-branchy instead of using "early returns".
-------------
Marked as reviewed by stefank (Committer).
PR Review:
https://git.openjdk.org/valhalla/pull/2254#pullrequestreview-3996801590
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2254#discussion_r2979482462
