[10] RFR 8186469 MethodHandle.invokeWithArguments jumbo-arity

[Paul Sandoz]

> On 22 Aug 2017, at 09:52, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:
> 
> Overall, looks good.
> 
> Some comments/questions:
> 
> src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java:
> 
> +            if (elemType == null)  // must be array type
> +                return asFixedArity().invokeWithArguments(arguments);
> 
> Since it's in AsVarargsCollector, is it possible that the last argument is not an array?
> 

I don’t see how that can happen, the arrayType passed to the constructor of AsVarargsCollector should be the same as the last parameter type.


> 
> +            Object collArgs = (elemType == Object[].class)
> +                ? new Object[collected] : Array.newInstance(arrayType.getComponentType(), collected);
> 
> I'm curious why there's a special case for Object[].class.
> Also, should Object.class be used instead of Object[].class?
> 

Yeah, i suspect John was trying to avoid an unnecessary call to Array.newInstance.

I’ll clean this up and update the test (thanks!).

Paul.


> test/java/lang/invoke/BigArityTest.java:
> 
> The test doesn't cover cases with uncollected arguments (uncollected > 0). It's easy to extend it, e.g. [1].
> 
> Best regards,
> Vladimir Ivanov
> 
> [1]
> 
> diff --git a/test/java/lang/invoke/BigArityTest.java b/test/java/lang/invoke/BigArityTest.java
> --- a/test/java/lang/invoke/BigArityTest.java
> +++ b/test/java/lang/invoke/BigArityTest.java
> @@ -77,9 +77,36 @@
>         }
>         return hashArguments(wrappedArgs);
>     }
> +
> +    static Object hashArguments1(Object o, Object... args) {
> +        Object[] arr = new Object[args.length+1];
> +        arr[0] = 0;
> +        System.arraycopy(args, 0, arr, 1, args.length);
> +        return Objects.hash(arr);
> +    }
> +    static Object hashArguments1(int i0, int... args) {
> +        Object[] wrappedArgs = new Object[args.length+1];
> +        wrappedArgs[0] = i0;
> +        for (int i = 0; i < args.length; i++) {
> +            wrappedArgs[i+1] = args[i];
> +        }
> +        return hashArguments(wrappedArgs);
> +    }
> +    static Object hashArguments1(long l0, long... args) {
> +        Object[] wrappedArgs = new Object[args.length+1];
> +        wrappedArgs[0] = l0;
> +        for (int i = 0; i < args.length; i++) {
> +            wrappedArgs[i+1] = (int) args[i];
> +        }
> +        return hashArguments(wrappedArgs);
> +    }
> +
>     static final MethodHandle MH_hashArguments_VA;
>     static final MethodHandle MH_hashArguments_IVA;
>     static final MethodHandle MH_hashArguments_JVA;
> +    static final MethodHandle MH_hashArguments1_VA;
> +    static final MethodHandle MH_hashArguments1_IVA;
> +    static final MethodHandle MH_hashArguments1_JVA;
>     static {
>         try {
>             MH_hashArguments_VA =
> @@ -91,6 +118,15 @@
>             MH_hashArguments_JVA =
>                 MethodHandles.lookup().unreflect(
> BigArityTest.class.getDeclaredMethod("hashArguments", long[].class));
> +            MH_hashArguments1_VA =
> +                    MethodHandles.lookup().unreflect(
> + BigArityTest.class.getDeclaredMethod("hashArguments1", Object.class, Object[].class));
> +            MH_hashArguments1_IVA =
> +                    MethodHandles.lookup().unreflect(
> + BigArityTest.class.getDeclaredMethod("hashArguments1", int.class, int[].class));
> +            MH_hashArguments1_JVA =
> +                    MethodHandles.lookup().unreflect(
> + BigArityTest.class.getDeclaredMethod("hashArguments1", long.class, long[].class));
>         } catch (ReflectiveOperationException ex) {
>             throw new Error(ex);
>         }
> @@ -377,11 +413,19 @@
>             Object r0 = Objects.hash(args);
>             Object r = MH_hashArguments_VA.invokeWithArguments(args);
>             assertEquals("jumbo arity="+arity, r0, r);
> +
> +            assertEquals("jumbo arity="+arity, r0, MH_hashArguments1_VA.invokeWithArguments(args));
> +
>             // use primitive typed argument lists also:
>             Object r1 = MH_hashArguments_IVA.invokeWithArguments(args);
>             assertEquals("jumbo int arity="+arity, r0, r1);
> +
> +            assertEquals("jumbo int arity="+arity, r0, MH_hashArguments1_IVA.invokeWithArguments(args));
> +
>             Object r2 = MH_hashArguments_JVA.invokeWithArguments(args);
>             assertEquals("jumbo long arity="+arity, r0, r2);
> +
> +            assertEquals("jumbo long arity="+arity, r0, MH_hashArguments1_JVA.invokeWithArguments(args));
>         }
>         System.out.println("  jumbo arities = "+arities);
>     }
> 
> 
> On 8/19/17 2:11 AM, Paul Sandoz wrote:
>> Hi,
>> Please review this API enhancement for MethodHandle.invokeWithArguments to support jumbo-arity and for
>> BSM invocation to be specified in terms of MethodHandle.invokeWithArguments:
>>   http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8185992-invoke-with-arguments-jumbo/webrev/
>> This patch is brought to you by John Rose and initially reviewed by myself.
>> It’s low hanging fruit that is the first deliverable from the JEP 309: Dynamic Class-File Constants [1] effort.
>> A CSR is underway.
>> Paul.
>> [1] http://openjdk.java.net/jeps/309