RFR (XS): 8224110 [lworld] Reflective method invocation fails with assert: "illegal type"

[David Simms]

I agree, and you are correct, the scope of JDK-8226393 needs widening to 
be a more encompassing of inline types. Flattened fields vs indirect 
haven't been addressed at all, the initial "ValueWithJni.java" test 
needs a serious revisit.

Frederic and I will probably need to look at the best way forward for 
the VM code, since it is not obvious we want polymorphic "oop 
arrayOop::obj_at(int index)", given one needs to allocate a buffer for 
the flattened variant. I can see there are use cases where we want to 
maintain flatten data as is.

Thanks for looking at the patch !

/D

On 2019-06-19 17:59, Mandy Chung wrote:
> This patch looks okay for LW2.  I think this requires more thought
> whether valueArray-to-objArray conversion should be done in JNIHandles
> rather than patching specific API points or other alternatives.
> When we begin looking at JNI support for inline class, that may
> be time to revisit this.
>
> Mandy
>
> On 6/19/19 1:59 AM, David Simms wrote:
>>
>> Update webrev: http://cr.openjdk.java.net/~dsimms/valhalla/8224110/
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8224110
>>
>> So I've updated the webrev, found two uses "jobjectArray" passed into 
>> the JVM from user code, where a inline type array may be in used as 
>> varargs: "JVM_InvokeMethod" and "JVM_NewInstanceFromConstructor".
>>
>> Added a helper method for converting to objArray to keep the rest of 
>> the VM code happy.
>>
>> /D
>>
>> On 2019-06-13 08:48, David Simms wrote:
>>>
>>> Thanks for pointing out this is a more general issue...your counter 
>>> use case is useful.
>>>
>>> Will have a look around for other interesting cases. For LW2 there 
>>> might be an initial hack to convert to objArrayOop.
>>>
>>> There is an issue in the VM now where, yes java inline type arrays 
>>> are co-variant but objArrayOop and valueArrayOop do not share the 
>>> same type relationship, and maybe they should. I.e. VM arrayOop 
>>> get/put elements methods (yeah there is polymorphism in C++). 
>>> Unclear we would win anything in terms of performance or heap for VM 
>>> runtime code. VM runtime always handles oops (drastically reduces 
>>> code changes), meaning conversion to objArrayOop is actually fine.
>>>
>>> JIT intrinsics will benefit if we allow inline type array.
>>>
>>> I'll rework the patch, and do a little more digging. Thanks for the 
>>> feedback guys.
>>>
>>> /D
>>>
>>> On 12/06/2019 21:26, Roger Riggs wrote:
>>>> Hi David,
>>>>
>>>> Is that unique to j.l.invoke?
>>>> There are plenty uses of varargs where the arguments
>>>> are the same type. <T> List.of(<T>... strings), for example.
>>>> There is the ambiguity for a single argument that the compiler 
>>>> warns about
>>>> as in the example that developers have to deal with.
>>>>
>>>> We should avoid exceptions, they require more specification (and 
>>>> warts)
>>>> than being able to just use the whole spec.
>>>> Getting arguments to this kind of method or constructor efficiently 
>>>> is important.
>>>>
>>>> $.02, Roger
>>>>
>>>>
>>>> On 6/12/19 12:53 PM, David Simms wrote:
>>>>>
>>>>> Updated the bug with comment, I was about say, sure let's use 
>>>>> inline type array if you want. But the use cases are thin:
>>>>>
>>>>> "The more I think about it the less I think we should allow inline 
>>>>> type array, not useless all the method args the same inline type, ie:
>>>>>
>>>>> "test(MyValue3 v1, MyValue3 v2, MyValue3 v3)", then passing 
>>>>> MyValue3[] would make sense...but only when all the arg types are 
>>>>> the same...seems like an edge case"
>>>>>
>>>>> All the parameter types have to be the same...is that worth a 
>>>>> special case ?
>>>>>
>>>>>
>>>>> On 12/06/2019 16:52, Roger Riggs wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Would this be just a temporary restriction?
>>>>>> I could see the benefits of being able to use an inline array 
>>>>>> class for varargs.
>>>>>>
>>>>>> ?, Roger
>>>>>>
>>>>>> On 06/12/2019 09:27 AM, David Simms wrote:
>>>>>>> Greetings,
>>>>>>>
>>>>>>> Please review this small fix to prevent "inline type" arrays 
>>>>>>> being sent directly "JVM_InvokeMethod" and 
>>>>>>> "JVM_NewInstanceFromConstructor". I don't believe it is the 
>>>>>>> intention of these methods to receive such a type, in the 
>>>>>>> original problem, there was a javac warning of passing arg[0] 
>>>>>>> implicitly converted to args array.
>>>>>>>
>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8224110
>>>>>>>
>>>>>>> Webrev: http://cr.openjdk.java.net/~dsimms/valhalla/8224110/
>>>>>>>
>>>>>>>
>>>>>>
>>>>
>