[9] RFR (S): 8148994: Replacing MH::invokeBasic with a direct call breaks LF customization

[John Rose]

Better!  You moved some inlining policy out of the VM and into the JDK runtime.
Reviewed.

Some loose brainstorming here:

We are creating MH nodes that provide hints to the JIT inlining policy.
The hint must vary over time as the profile evolves.  The CMH does this
indirectly via generating a @DontInline LF, with the LF patchable.
Another way to do it would be to have a @Stable MH pointer
initially null, but which would go non-null when an inlineable
MH is determined.  Another way would be to use a mutable CS.
The handshake might need to include a "push" notification to
the JIT that something has changed and recompilation should
be attempted.  The MCS can do this.  Or perhaps the push should
happen after some sort of further countdown.

— John

> On Feb 12, 2016, at 9:15 AM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:
> 
> Any reviews, please?
> 
> Best regards,
> Vladimir Ivanov
> 
> On 2/5/16 7:29 PM, Vladimir Ivanov wrote:
>> Thanks, John.
>> 
>> On 2/4/16 9:52 PM, John Rose wrote:
>>> Looks good. Except I don't recall how is_inline interacts, if at all,
>>> with customization. It's not clear what is being gated, and which
>>> optimizations get let through.
>> Inlining through MH::invokeBasic doesn't happen when there's @DontInline
>> on the target. It is the case for GWT which has CountingWrapper on both
>> paths. It starts with a reinvoker.dontInline LF and then switches to a
>> reinvoker LF (after 30 invocations). If a compilation happens when
>> reinvoker.dontInline is used, a direct call is generated and even if the
>> branch becomes hot, all the calls goes through reinvoker.dontInline.
>> Update to reinvoker LF isn't visible unless the dispatch goes through
>> MH::invokeBasic linker.
>> 
>> I had an impression that I introduced LF customization logic to the
>> non-inlined case in GWT, but I can't find it in the code. So, it seems
>> the regression is caused by repeated checks performed by
>> reinvoker.dontInline LF and not by LF customization missed in generated
>> code.
>> 
>> Also, I stumbled upon a quirk in inlining policy: inlining through
>> linkers can't be delayed. It happens when JIT hits some limit on IR
>> size. So, right now inlining can stop in the middle of MH chain and a
>> linker call is generated.
>> 
>> But after JDK-8072008 there's no problem with delaying inlining. C2 can
>> decide whether to keep the direct call or inline through it. So, I
>> enabled late inlining for all linkers. (Surprisingly, no significant
>> performance difference on nashorn.)
>> 
>> So, I decided to keep invokeBasic linker call replacement for now, but
>> piggypack LF customization on the stuck counting LF.
>> 
>> Updated webrev:
>>   http://cr.openjdk.java.net/~vlivanov/8148994/webrev.01/hotspot
>>   http://cr.openjdk.java.net/~vlivanov/8148994/webrev.01/jdk
>> 
>> * enabled late inlining for linkers;
>> * add LF customization check in CountingWrapper::countDown, so when
>> counting is stuck, the target will be customized.
>> 
>> Testing: octane, JPRT.
>> 
>> I'm working on JDK-8071793 [1] and will reconsider how LF customization
>> on non-inlined branch in GWTs works given there is a more JVM-friendly
>> mechanism to control inlining. With the proposed fix
>> reinvoker.dontInline is never customized (only the target it points to)
>> and serves as a trampoline between GWT & target LF. It would be better
>> to jump right to the customized LF, if non-inlined branch becomes hot.
>> 
>> Best regards,
>> Vladimir Ivanov
>> 
>> [1] https://bugs.openjdk.java.net/browse/JDK-8071793
>>     "Prune rarely called invokedynamic call sites during inlining"
>>> 
>>> Maybe a comment about that would be useful
>>> Reviewed.
>>> 
>>> – John
>>> 
>>>> On Feb 4, 2016, at 7:51 AM, Vladimir Ivanov
>>>> <vladimir.x.ivanov at oracle.com> wrote:
>>>> 
>>>> http://cr.openjdk.java.net/~vlivanov/8148994/webrev.00
>>>> https://bugs.openjdk.java.net/browse/JDK-8148994
>>>> 
>>>> JDK-8072008 [1] introduced an optimization to bypass linker calls in
>>>> generated code when target method can't be inlined.
>>>> 
>>>> Unfortunately, it doesn't work well with MH::invokeBasic(), because
>>>> it clashes with LambdaForm customization: if a call is not inlined,
>>>> it is important for performance to update (customize) corresponding
>>>> LambdaForm on-the-fly. MH::invokeBasic() linker does MH -> LF ->
>>>> Method* -> _from_compiled_entry traversal on every invocation, while
>>>> direct call points right to the _from_compiled_entry. So, any
>>>> LambdaForm updates aren't visible until recompilation.
>>>> 
>>>> The fix is to keep MH::invokeBasic linker, so up-to-date LambdaForm
>>>> instance is used on every invocation.
>>>> 
>>>> Also, cleaned up relevant tests a bit.
>>>> 
>>>> Testing: octane, JPRT.
>>>> 
>>>> Best regards,
>>>> Vladimir Ivanov
>>>> 
>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8072008
>>>>    "Emit direct call instead of linkTo* for recursive
>>>> indy/MH.invoke* calls"