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();
