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

[Mandy Chung]

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/
>>>>>>
>>>>>>
>>>>>
>>>