[lworld] RFR: lworld aaload/aastore
karen.kinnear at oracle.com
Fri Mar 23 15:27:47 UTC 2018
Thank you so much for implementing this and clearly describing the JVMS changes and behavior we agreed to
prototype. And I am grateful for all the test cases and experiments with array behavior with
The code looks good. (If you already pushed it great - I didn’t see a notification).
1. interpreterRuntime.cpp 496 - value_array_load - you changed the assertion check, so the message no longer matches
There are two places that say // Move superclass into pax - but I think they are moving the element class?
lines 1260 and 1194
3. So my understanding is that: all arrays with a value type element are flattenable
and the flatten decision is made in ValueKlass::array_klass_impl, so you either get a ValueArrayKlass - always
flattened or an ObjArrayKlass with element type value type - so keeps non-nullable semantics.
Is that accurate?
So test_flat_array_oop is a flattened test and all ValueTypeArrays set the vt_array bit.
looking forward to the profiling info.
> On Mar 21, 2018, at 9:28 AM, David Simms <david.simms at oracle.com> wrote:
> Updated version of aaload/aastore and copy_array for the interpreter...
> Assumes the current thinking that all value type arrays are flat, but are covariant with Object array and interface arrays:
> * aaload/aastore: works with both objArrayOop and valueArrayOop (flat)
> o currently assumes always flat semantics for value type element
> class even for objArrayOop (if value deemed unsuitable as flat),
> i.e. no null-ability
> o objArrayOop with values works as vanilla array, i.e. has
> null-ability, for non-value type classes
> * aaload JVMS changes:
> o If source array element type is value class (flat array
> semantics), the result is never null, but equivalent to the
> "defaultvalue" bytecode.
> * aastore JVMS changes:
> o May NPE if the target array element type is value class (flat
> array semantics)
> o May OOM if the source value is "buffered" and target array is
> element type is not value class (ref array semantics)
> * covariance
> o V <: I: Object <: Object where V = value type class, and I
> = any one of V's interface super types.
> + E.g. "__ByValue Point implements Comparable", therefore:
> Point <: Comparable
> + instanceof/checkcast, functions as described above
> + System.arraycopy() (interpreter implementation) now accepts
> as described above
> # May NPE when attempting to store null into an array with
> element type of value class (flat array semantics)
> There are number of tests added to "ValueTypeArray.java", including some sanity test of some "java.util" array related classes. Collections are a larger topic which the test does not try to address fully. But the sanity tests do raise a few interesting observations when fitting value arrays into current JDK collections:
> * ArrayList and ArrayDeque specifically use "Object elementData",
> and accepts no subtype (see "ArrayList(Collection<? extends E> c)",
> will even copy the source array if not Object)
> o So given System.arraycopy from value array to object array now
> works, as does aaload/store of a value to and from Object array,
> it all works (in so far as the sanity check tests). Even
> ArrayList.remove(), since the backing store is always a
> reference array.
> * Arrays.toList() produces its own ArrayList with a backing array of
> the sharp type, so potentially a value type array without
> null-ability characteristics
> o ...this is fine, doesn't implement "remove()".
> Given what I said about ArrayList backing being Object ("Collection.toArray()" must return component type Object), and the API allowing nulls, our current javac behavior is a little weird. Check this code out:
> ArrayList<MyInt> aList = new ArrayList<MyInt>(Arrays.asList(copyMyInts));
> aList.add((MyInt)getNull()); // <<< Some issues here, should this be legal ? With "ArrayList<>", yes but maybe not with "ArrayList<Value-Type>"
> Currently javac is inserting "java/util/Objects.requireNonNull". At first I wanted to argue that check-cast should handle null with value class automatically. But wait, generic erasure means I see "ArrayList.add(LObject;)" in the VM, so null is perfectly fine...but the intention is "ArrayList<MyInt>.add(LMyInt;)...so my bad ?
More information about the valhalla-dev