Re: RFR(S): 8157225: adopt method handle for array length getter in BeanLinker

2016-05-18 Thread Michael Haupt 
Hi Attila,

thanks for your comments.

> Am 18.05.2016 um 16:11 schrieb Attila Szegedi :
> There’s a bit of an issue with this. Namely, the old GET_ARRAY_LENGTH being 
> the unreflection of j.l.reflect.Array.getLength() worked for any array type, 
> so it was always accompanied with ValidationType.IS_ARRAY. I believe this is 
> wrong now and can cause ClassCastException at runtime (I say “believe” as I 
> can’t currently build jdk 9 so can’t confirm). This could be caught by a test 
> that handles two arrays of different types in the same call site, e.g. with 
> Nashorn:
> 
>  var intArray = Java.type(“int[]”)
>  var doubleArray = Java.type(“double[]”)
>  var arrs = [new intArray(0), new doubleArray(0)]
>  for (var i in arrs) arrs[i].length

Confirmed:

$ jjs
jjs> var intArray = Java.type("int[]")
jjs> var doubleArray = Java.type("double[]")
jjs> var arrs = [new intArray(0), new doubleArray(0)]
jjs> for (var i in arrs) arrs[i].length
java.lang.ClassCastException: Cannot cast [D to [I

> Since the MethodHandles.arrayLength is typed to the concrete array class, it 
> should be used with ValidationType.EXACT_CLASS.

Also confirmed:

$ jjs
jjs> var intArray = Java.type("int[]")
jjs> var doubleArray = Java.type("double[]")
jjs> var arrs = [new intArray(0), new doubleArray(0)]
jjs> for (var i in arrs) arrs[i].length
0

> This will obviously cause call sites taking length of different kinds of 
> arrays more polymorphic… FWIW, the IS_ARRAY validation type is probably 
> obsolete now as it was only used for array length getters anyway.

Yes; there are two uses of IS_ARRAY that can be replaced with EXACT_CLASS ...

> The polymorphism could be somewhat alleviated by having stable validation for 
> all arrays of reference types. Maybe have one 
> MethodHandles.arrayLength(Object[].class) handle cached to use with all 
> arrays of reference types, and use a Validator(Object[].class, 
> ValidationType.INSTANCE_OF) with it. Then you’d have stable linkage that can 
> take the length of any reference-typed array, but you’d still trigger 
> relinking for primitive-typed arrays.

... but shouldn't, right.

I've filed a bug to adopt the quick fix with EXACT_CLASS [1], and another to 
deal with the relinking problem [2]. The solution you suggest should be 
possible; maybe the IS_ARRAY abstraction can be reused for this in a way that 
avoids relinking for primitive arrays by having a PIC-like structure.

Best,

Michael

[1] https://bugs.openjdk.java.net/browse/JDK-8157250
[2] https://bugs.openjdk.java.net/browse/JDK-8157251


-- 

 
Dr. Michael Haupt | Principal Member of Technical Staff
Phone: +49 331 200 7277 | Fax: +49 331 200 7561
Oracle Java Platform Group | LangTools Team | Nashorn
Oracle Deutschland B.V. & Co. KG | Schiffbauergasse 14 | 14467 Potsdam, Germany

ORACLE Deutschland B.V. & Co. KG | Hauptverwaltung: Riesstraße 25, D-80992 
München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V. | Hertogswetering 163/167, 
3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
  Oracle is committed to developing 
practices and products that help protect the environment

Re: RFR(S): 8157225: adopt method handle for array length getter in BeanLinker

2016-05-18 Thread Attila Szegedi 
There’s a bit of an issue with this. Namely, the old GET_ARRAY_LENGTH being the 
unreflection of j.l.reflect.Array.getLength() worked for any array type, so it 
was always accompanied with ValidationType.IS_ARRAY. I believe this is wrong 
now and can cause ClassCastException at runtime (I say “believe” as I can’t 
currently build jdk 9 so can’t confirm). This could be caught by a test that 
handles two arrays of different types in the same call site, e.g. with Nashorn:

  var intArray = Java.type(“int[]”)
  var doubleArray = Java.type(“double[]”)
  var arrs = [new intArray(0), new doubleArray(0)]
  for (var i in arrs) arrs[i].length

Since the MethodHandles.arrayLength is typed to the concrete array class, it 
should be used with ValidationType.EXACT_CLASS. 

This will obviously cause call sites taking length of different kinds of arrays 
more polymorphic… FWIW, the IS_ARRAY validation type is probably obsolete now 
as it was only used for array length getters anyway.

The polymorphism could be somewhat alleviated by having stable validation for 
all arrays of reference types. Maybe have one 
MethodHandles.arrayLength(Object[].class) handle cached to use with all arrays 
of reference types, and use a Validator(Object[].class, 
ValidationType.INSTANCE_OF) with it. Then you’d have stable linkage that can 
take the length of any reference-typed array, but you’d still trigger relinking 
for primitive-typed arrays.

Attila.
 
> On 18 May 2016, at 11:25, Michael Haupt  wrote:
> 
> Dear all,
> 
> please review this change.
> RFE: https://bugs.openjdk.java.net/browse/JDK-8157225
> Webrev: http://cr.openjdk.java.net/~mhaupt/8157225/webrev.00/
> 
> Thanks,
> 
> Michael
> 
> -- 
> 
> 
> Dr. Michael Haupt | Principal Member of Technical Staff
> Phone: +49 331 200 7277 | Fax: +49 331 200 7561
> Oracle Java Platform Group | LangTools Team | Nashorn
> Oracle Deutschland B.V. & Co. KG | Schiffbauergasse 14 | 14467 Potsdam, 
> Germany
> 
> ORACLE Deutschland B.V. & Co. KG | Hauptverwaltung: Riesstraße 25, D-80992 
> München
> Registergericht: Amtsgericht München, HRA 95603
> 
> Komplementärin: ORACLE Deutschland Verwaltung B.V. | Hertogswetering 163/167, 
> 3543 AS Utrecht, Niederlande
> Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697
> Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
> Oracle is committed to developing 
> practices and products that help protect the environment
>

Re: RFR(S): 8157225: adopt method handle for array length getter in BeanLinker

2016-05-18 Thread Hannes Wallnoefer 

+1

Am 2016-05-18 um 11:25 schrieb Michael Haupt:

Dear all,

please review this change.
RFE: https://bugs.openjdk.java.net/browse/JDK-8157225
Webrev: http://cr.openjdk.java.net/~mhaupt/8157225/webrev.00/

Thanks,

Michael

Re: RFR(S): 8157225: adopt method handle for array length getter in BeanLinker

2016-05-18 Thread Sundararajan Athijegannathan 
+1

Nice!

-Sundar


On 5/18/2016 2:55 PM, Michael Haupt wrote:
> Dear all,
>
> please review this change.
> RFE: https://bugs.openjdk.java.net/browse/JDK-8157225
> Webrev: http://cr.openjdk.java.net/~mhaupt/8157225/webrev.00/
>
> Thanks,
>
> Michael
>

RFR(S): 8157225: adopt method handle for array length getter in BeanLinker

2016-05-18 Thread Michael Haupt 
Dear all,

please review this change.
RFE: https://bugs.openjdk.java.net/browse/JDK-8157225
Webrev: http://cr.openjdk.java.net/~mhaupt/8157225/webrev.00/

Thanks,

Michael

-- 

 
Dr. Michael Haupt | Principal Member of Technical Staff
Phone: +49 331 200 7277 | Fax: +49 331 200 7561
Oracle Java Platform Group | LangTools Team | Nashorn
Oracle Deutschland B.V. & Co. KG | Schiffbauergasse 14 | 14467 Potsdam, Germany

ORACLE Deutschland B.V. & Co. KG | Hauptverwaltung: Riesstraße 25, D-80992 
München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V. | Hertogswetering 163/167, 
3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
  Oracle is committed to developing 
practices and products that help protect the environment