Re: RFR:8157251:BeanLinker relinks array length operations for array types

[Priya Lakshmi Muthuswamy]

Hi Attila,

Thanks for the review. I just felt in the case of primitive types there 
will be relinking.
I tried with the below fix.
In the case object types, I see relinking when we use 
Validation.INSTANCE_OF. But works fine without relinking when we use 
Validation.IS_ARRAY

if(clazz.isArray()) {
    // Some languages won't have a notion of manipulating collections. 
Exposing "length" on arrays as an // explicit property is beneficial for 
them. if (clazz.getComponentType().isPrimitive()) {
        setPropertyGetter("length", MethodHandles.arrayLength(clazz), 
ValidationType.EXACT_CLASS);
    }else {
        setPropertyGetter("length", MethodHandles.arrayLength(Object[].class), 
ValidationType.IS_ARRAY);
    }

Thanks,
Priya


On 1/9/2018 7:51 PM, Attila Szegedi wrote:
This effectively reverts the combined 
<https://bugs.openjdk.java.net/browse/JDK-8157225> and 
<https://bugs.openjdk.java.net/browse/JDK-8157250>.

That was not the intent of 8157251, though.

The intent of 8157251 was to use MethodHandles.arrayLength(Object[].class) for 
all arrays with components of reference type (and use 
ValidationType.INSTANCE_OF for it, as all such arrays will be instance-of 
Object[] through instance-of semantics for array types) and leave the linking 
of arrays with primitive component types as they are today.

So, basically, something like:

if(clazz.isArray()) {
     // Some languages won't have a notion of manipulating collections. Exposing 
"length" on arrays as an
     // explicit property is beneficial for them.
     if (clazz.getComponentType().isPrimitive()) {
         setPropertyGetter("length", MethodHandles.arrayLength(clazz), 
ValidationType.EXACT_CLASS);
     } else {
         setPropertyGetter("length", MethodHandles.arrayLength(Object[].class), 
ValidationType.INSTANCE_OF);
     }

Obviously, this is again a tradeoff between linkage stability and performance, 
except that with this solution we don’t sacrifice any performance and we still 
increase linkage stability. Going back to Array.getLength does further increase 
linkage stability, but *speculatively* it doesn’t intrinsify to as lean code as 
MethodHandles.arrayLength does (it certainly erases the type information of the 
argument); someone with time on their hands should probably run some 
PrintAssembly tests with -XX:-TieredCompilation to see whether this is true (if 
not, then 8157225 and 8157250 were done for nothing and this patch undoes them 
anyway).

In any case, I don’t have a too strong opinion about this; I don’t mind even if 
this ends up being a reversal of 8157225; it’s just weird that we have 
arrayLength method handle intrinsics and not use them.

Attila.


On Jan 9, 2018, at 8:00 AM, Priya Lakshmi Muthuswamy 
<priya.lakshmi.muthusw...@oracle.com> wrote:

Hi,

Please review JDK-8157251 : BeanLinker relinks array length operations for 
array types

JBS : https://bugs.openjdk.java.net/browse/JDK-8157251
webrev : http://cr.openjdk.java.net/~pmuthuswamy/8157251/webrev.00/

Thanks,
Priya