Review of MH/LF patches in the review queue (original) (raw)
Paul Sandoz paul.sandoz at oracle.com
Fri Apr 4 11:33:42 UTC 2014
- Previous message: RFR: 8039256 Add sun/jvmstat/monitor/MonitoredVm/CR6672135.java to ProblemList.txt
- Next message: Review of MH/LF patches in the review queue
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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();- Previous message: RFR: 8039256 Add sun/jvmstat/monitor/MonitoredVm/CR6672135.java to ProblemList.txt
- Next message: Review of MH/LF patches in the review queue
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]