[9] RFR (M): 8037209: Improvements and cleanups to bytecode assembly for lambda forms

[Vladimir Ivanov]

Paul,

Unfortunately, additional profiling doesn't work for Accessor.checkCast 
case. The problem is Accessor.checkCast is called from multiple places, 
so type profile is very likely to be polluted. And it kills the benefits.

I don't think MethodType helps with profiling in any way. It represents 
type info which is necessary for correctness checks. Profiling collects 
more fine-grained information (e.g. exact types, values).

Regarding redundant null check, do you have a test case so I can play 
with it myself?

Best regards,
Vladimir Ivanov

On 4/1/14 1:55 PM, Paul Sandoz wrote:
> On Mar 14, 2014, at 5:36 PM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> 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 at 1 (line 3253)
>                                                 ; - java.lang.invoke.InstanceFieldHandle::checkCast at 2 (line 133)
>                                                 ; - java.lang.invoke.InstanceFieldHandle::set at 19 (line 153)
>                                                 ; - java.lang.invoke.VarHandle::set at 21 (line 127)
>                                                 ; - VarHandleTest::testLoopOne at 8 (line 157)
>                                                 ;   {runtime_call}
>   0x000000010d050fb0: callq  0x000000010c39d330  ;*ifnull
>                                                 ; - java.lang.Class::cast at 1 (line 3253)
>                                                 ; - java.lang.invoke.InstanceFieldHandle::checkCast at 2 (line 133)
>                                                 ; - java.lang.invoke.InstanceFieldHandle::set at 19 (line 153)
>                                                 ; - java.lang.invoke.VarHandle::set at 21 (line 127)
>                                                 ; - VarHandleTest::testLoopOne at 8 (line 157)
>                                                 ;   {runtime_call}
>