[10] RFR 8186469 MethodHandle.invokeWithArguments jumbo-arity

[stanislav lukyanov]

Hi Paul,

// a non-Reviewer here

Some Javadoc nits:

- package-info.Java
 > optionally, between 1 and 251 additional static arguments taken from 
the constant pool
Looks like "between 1 and 251" part can be removed.
It could also say "251 for non-varargs, unlimited for varargs" but it is 
already discussed in the
"types of bootstrap methods" section, so can be omitted here.

 > In all cases, bootstrap method invocation is as if by
 > {@link java.lang.invoke.MethodHandle#invokeWithArguments 
MethodHandle.invokeWithArguments},
 > (This is also equivalent to
 > {@linkplain java.lang.invoke.MethodHandle#invoke generic invocation}
 > if the number of arguments is small enough.)
Now in two places both invoke and invokeWithArguments are mentioned as 
equivalent "if the number of arguments is small enough",
and some parts only talk about invoke.
Can all these references be changed to invokeWithArguments, for 
simplicity? I guess the invokedynamic
specification in JVMS will also only use invokeWithArguments for BSM as 
well.

 > For an {@code invokedynamic} instruction (...)
The context is all about invokedynamic, so this is probably not needed.

- MethodType.java
void as a "sentinel type" seems a bit clunky, even though it seems to be 
safe when used
in `lastParameterType().getComponentType()` and I can't think of many 
use cases besides that...
If the `lastParameterType().getComponentType()` is the only use case, 
does the method really needs
to be public? If there are more use cases, is void always safe and not 
confusing?
Would Optional be better?

Also,
 > @since 9
@since 10 ? :)

- MethodHandle.java
 > <li>Collect the {@code N} elements of the array as a logical
 >     argument list, each argument statically typed as an {@code 
Object}. </li>
 > <li>Determine, as {@code M}, the parameter count of the type of this
 >     method handle. </li>
 > <li>Determine the general type {@code TN} of {@code N} arguments as
 >     as {@code TN=MethodType.genericMethodType(N)}.</li>
 > <li>If {@code N} is greater than {@code M}, perform the following
 >     checks and actions to shorten the logical argument list: <ul>
 >     <li>Check that this method handle has variable arity with a
 >         {@linkplain MethodType#lastParameterType trailing parameter}
 >         of some array type {@code A[]}.  If not, fail with a
 >         {@code WrongMethodTypeException}. </li>
 >     <li>Collect the trailing elements (there are {@code N-M+1} of them)
 >         from the logical argument list into a single array of
 >         type {@code A[]}, using {@code asType} conversions to
 >         convert each trailing argument to type {@code A}. </li>
 >     <li>If any of these conversions proves impossible, fail with a
 >         {@code WrongMethodTypeException}. </li>
 >     <li>Replace the logical arguments gathered into the array of
 >         type {@code A[]} with the array itself, thus shortening
 >         the argument list to length {@code M}. This final argument
 >         retains the static type {@code A[]}.</li>
 >     <li>Adjust the type {@code TN} by changing the {@code N}th
 >         parameter type from {@code Object} to {@code A[]}.
 >     </ul>
First, implied `genericMethodType(N)` still fails with N > 255. It could 
safely be `genericMethodType(M)` though.

Second, it's unfortunate that varargs collection has to be 
re-implemented in the spec
for `invokeWithArguments` instead of reusing what `asVarargsCollector` has.
Amusingly, the current spec is almost good enough for unlimited 
invokeWithArguments except for
the IAE from `genericMethodType(N)` when N > 255 (*). So, a hackish 
addition like
     "Unlike actual calls, the implied `genericMethodType` and `asType` 
don't fail
     when a method type with too many parameters is created."
would actually be sufficient to allow the new behavior. Maybe that's the 
way to go?

(*) there is also a corner case of `asType(genericMethodType(255))` that 
throws WMTE - see JDK-8177275.

 > (...) the excess arguments, starting at the position of the trailing
 > array argument, will be gathered (if possible, as if by {@code 
asType} conversions)
It doesn't seem necessary for the spec to be verbose about that 
"regular" varargs processing can
be reused for M < N < MAX_SAFE and the special processing is only used 
for N > MAX_SAFE.
The observable behavior is the same for any N > M, so introducing 
MAX_SAFE seems redundant.

Thanks,
Stas

On 19.08.2017 4:41, 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
>
>