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

[Vladimir Ivanov]

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"