Review of MH/LF patches in the review queue

[Paul Sandoz]

Hi,

Reviews of two patches in the queue.

Paul.


http://cr.openjdk.java.net/~vlivanov/8037209/webrev.02/

Looks good, though I don't claim to understand all the nuances around casts and JVM/bytecode correctness. Minor stuff below.

InvokerByteCodeGenerator:
--

Unused method:

    static boolean match(MemberName member, MethodHandle fn) {
        if (member == null || fn == null)  return false;
        return member.equals(fn.internalMemberName());
    }


MethodHandleImpl
--

If you are gonna remove the weakly typed wrapper methods for array access, you might as well remove USE_WEAKLY_TYPED_ARRAY_ACCESSORS and it's use?

-         // Weakly typed wrappers of Object[] accessors:
-         static Object  getElementL(Object    a, int i)            { return getElementL((Object[])a, i); }
-         static void    setElementL(Object    a, int i, Object  x) {        setElementL((Object[]) a, i, x); }
-         static Object  getElementL(Object   arrayClass, Object a, int i)             { return getElementL((Class<?>) arrayClass, (Object[])a, i); }
-         static void    setElementL(Object   arrayClass, Object a, int i, Object x)   {        setElementL((Class<?>) arrayClass, (Object[])a, i, x); }
- 

Or otherwise for expediency just leave it in until the array improvements patch follows? IMHO better to be consistent either way.


VerifyType
--

Typo in docs:

     * <p>
     * The primitive type 'void' does not interconvert with any other type,
     * even though it is legal to drop any type from the stack and "return void".
     * The stack effects, though are difference between void and any other type,
     * so it is safer to report a non-trivial conversion.

s/difference/different



http://cr.openjdk.java.net/~vlivanov/8038261/webrev.01/

To re-iterate this is a nice improvement over the previous approach.

InvokerByteCodeGenerator
--

For this specialization:

        if (defc == ArrayAccessor.class &&
                match(member, ArrayAccessor.OBJECT_ARRAY_GETTER)) {
            mv.visitInsn(Opcodes.AALOAD);
        } else if (defc == ArrayAccessor.class &&
                match(member, ArrayAccessor.OBJECT_ARRAY_SETTER)) {
            mv.visitInsn(Opcodes.AASTORE);
        } else if (member.isMethod()) {

IIUC all stuff on the stack is correctly placed for the substitution of the invokestatic instruction to ArrayAccessor.set/getElementL with an aastore/load instruction. This makes we wonder if there is a more general approach for direct or direct-like MHs to be visited and provide their own snippets of ASM producing code.

Is it worthwhile introducing such direct coupling for a specific case, as that tends to increase the inter-connective complexity of the code. How much performance gain is achieved?


The last two re-assigments are never used and are reassigned again at the top of the loop:

            // Update cached form name's info in case an intrinsic spanning multiple names was encountered.
            name = lambdaForm.names[i];
            member = name.function.member();
            rtype = name.function.methodType().returnType();