[lworld] RFR: 8366705: [lworld] Re-work of arrays meta-data [v4]

[David Simms]

On Wed, 3 Sep 2025 14:43:55 GMT, Frederic Parain <fparain at openjdk.org> wrote:

>> Since the removal of Q-types and the notion that nullability was not part of the Java type, there was an awkward situation because nullable arrays of value types and null free arrays of value types had each a different Java mirror when they were in fact supposed to have the same Java type.
>> In order to accommodate to the new situation, that arrays can have properties (nullability, flatness, atomicity, etc.) that are not part of their Java type, the 1-1 relationship between the *ArrayKlass and the Java mirror must be broken.
>> The proposed solution is to dedicate one instance of ObjArrayKlass to represent the Java type of the array in the JVM, and have this instance being the counterpart of the Java mirror of the array, and have several instances of RefArrayKlass and FlatArrayKlass that represent the refinements of the Java array type. Each RefArrayKlass/FlatArrayKlass encodes the characteristic of a Java array for a given element type and a set of properties.
>
> Frederic Parain has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 67 commits:
> 
>  - Merge branch 'array_klasses' of github.com:fparain/valhalla into array_klasses
>  - Forgot a TODO
>  - Small cleanup
>  - Merge remote-tracking branch 'upstream/lworld' into array_klasses
>  - Moved get_Klass() back to protected and updated usages
>  - Merge branch 'array_klasses' of github.com:fparain/valhalla into array_klasses
>  - Linked TODOs to JDK-8366668
>  - Multidim array fix
>  - Cleanup T_FLAT_ELEMENT related code
>  - Fix for isAssignableFrom + tests
>  - ... and 57 more: https://git.openjdk.org/valhalla/compare/22e9d5f5...527a17b6

Coleen beat me to most of the formatting issues and comment of deriving flatArray

Overall, this looking awesome.

One issue of concern: refArray and flatArray are distinct from any mainline code. But any cases of objArray changes in mainline, when merged may actually need changing to refArray. Especially if there is additional mainline code that does not overlap, i.e. there will no textual conflict and it will compile.

I took another pass at the PR looking assertions when need refined types. I think you have it covered. That does mean more assertions during merge testing, but they should obvious.

I'm not even going to complain about how textual conflict this PR will produce in future mainline merging, those are trivial to deal with.

If we were interested in catching merge problems earlier, we code rename objArray...but that feels extreme.

src/hotspot/share/ci/ciObjectFactory.cpp line 150:

> 148:   for (int i = T_BOOLEAN; i <= T_CONFLICT; i++) {
> 149:     BasicType t = (BasicType)i;
> 150:     if (type2name(t) != nullptr && t != T_FLAT_ELEMENT && !is_reference_type(t) &&

I can't remember know if we said we would attempt to remove `T_FLAT_ELEMENT` later ? If we still need it, as a "basic type" can "T_FLAT" cover both elements and fields ?

src/hotspot/share/oops/objArrayKlass.cpp line 159:

> 157: ArrayDescription ObjArrayKlass::array_layout_selection(Klass* element, ArrayProperties properties) {
> 158:   // TODO FIXME: the layout selection should take the array size in consideration
> 159:   // to avoid creation of arrays too big to be handled by the VM

There is a related bug (though it is old): [8233189](https://bugs.openjdk.org/browse/JDK-8233189), perhaps we need a new bug noted here (and relate the old one)

-------------

Marked as reviewed by dsimms (Committer).

PR Review: https://git.openjdk.org/valhalla/pull/1452#pullrequestreview-3183603436
PR Review Comment: https://git.openjdk.org/valhalla/pull/1452#discussion_r2320951128
PR Review Comment: https://git.openjdk.org/valhalla/pull/1452#discussion_r2321035022