> On 22 Aug 2017, at 09:52, Vladimir Ivanov <vladimir.x.iva...@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
