On Mar 14, 2014, at 5:36 PM, Vladimir Ivanov <[email protected]>
wrote:
>> Doh! crossed webrevs, thanks.
>>
>> Just had a quick look, this looks like a really nice improvement to the
>> array setter/getter support, definitely simplified. IIUC the mh.viewAsType
>> will now handle the appropriate casting. I believe it might reduce the
>> "ceremony" for array setter/getter MHs [1].
>>
>> I see there is a PROFILE_LEVEL option, by default set to 0, that results in
>> casts not being emitted:
>>
>> + if (VerifyType.isNullConversion(Object.class, pclass,
>> false)) {
>> + if (PROFILE_LEVEL > 0)
>> + emitReferenceCast(Object.class, arg);
>> + return;
>> + }
>> ...
>> + mv.visitLdcInsn(constantPlaceholder(cls));
>> + mv.visitTypeInsn(Opcodes.CHECKCAST, CLS);
>> + mv.visitInsn(Opcodes.SWAP);
>> + mv.visitMethodInsn(Opcodes.INVOKESTATIC, MHI, "castReference",
>> CLL_SIG, false);
>> + if (Object[].class.isAssignableFrom(cls))
>> + mv.visitTypeInsn(Opcodes.CHECKCAST, OBJARY);
>> + else if (PROFILE_LEVEL > 0)
>> + mv.visitTypeInsn(Opcodes.CHECKCAST, OBJ);
>>
>> Can you explain a bit the rational for that?
> These casts are redundant - they aren't required for bytecode correctness.
> The idea behind PROFILE_LEVEL is to provide more type information to
> JIT-compiler. Right now, type profiling occurs on every checkcast
> instruction. So, having these additional instructions we can feed C2 with
> more accurate information about types.
>
> Consider this as a hack to overcome some of the limitations of current
> profiling implementation in VM.
>
Apologies for the late reply this dropped off my radar...
Ah! i may have just had a minor epiphany :-)
So that is why in DirectMethodHandle there are casts for fields, via say
Accessor.checkCast?
@Override Object checkCast(Object obj) {
return fieldType.cast(obj);
}
if so could PROFILE_LEVEL be supported in that code too?
Perhaps the JIT could derive some profile information from the MethodType of
the MethodHandle?
I notice that in my experiments for enhanced access to instances of fields that
casts are almost optimized away but a null-check is left [*], which is also
seems redundant and could impact performance get/set of null values.
Paul.
[*]
0x000000010d050f70: test %r10d,%r10d
0x000000010d050f73: je 0x000000010d050f9d
...
0x000000010d050f9d: mov %rsi,%rbp
0x000000010d050fa0: mov %r10d,0x4(%rsp)
0x000000010d050fa5: mov $0xffffffad,%esi
0x000000010d050faa: nop
0x000000010d050fab: callq 0x000000010d0163e0 ; OopMap{rbp=Oop [4]=NarrowOop
off=112}
;*ifnull
; - java.lang.Class::cast@1
(line 3253)
; -
java.lang.invoke.InstanceFieldHandle::checkCast@2 (line 133)
; -
java.lang.invoke.InstanceFieldHandle::set@19 (line 153)
; -
java.lang.invoke.VarHandle::set@21 (line 127)
; - VarHandleTest::testLoopOne@8
(line 157)
; {runtime_call}
0x000000010d050fb0: callq 0x000000010c39d330 ;*ifnull
; - java.lang.Class::cast@1
(line 3253)
; -
java.lang.invoke.InstanceFieldHandle::checkCast@2 (line 133)
; -
java.lang.invoke.InstanceFieldHandle::set@19 (line 153)
; -
java.lang.invoke.VarHandle::set@21 (line 127)
; - VarHandleTest::testLoopOne@8
(line 157)
; {runtime_call}
