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

[forax at univ-mlv.fr]

Hi Vladimir,
Maybe i should have not talking about trying to recompile the code in case of LF customization :)

I'm mostly interested in specific corner cases and i think your patch is harmful for these cases.

There are reasons to not inline a call even if it can inlined, by example, if the inlining tree is too deep or if the function is recursive,
in those cases, the method handle is constant but the code is not inlined.

I may be wrong but for me if your patch is applied, the generated code will not emit a direct call as it should do.

Rémi

----- Mail original -----
> De: "Vladimir Ivanov" <vladimir.x.ivanov at oracle.com>
> À: "Remi Forax" <forax at univ-mlv.fr>
> Cc: "hotspot compiler" <hotspot-compiler-dev at openjdk.java.net>
> Envoyé: Vendredi 5 Février 2016 17:44:06
> Objet: Re: [9] RFR (S): 8148994: Replacing MH::invokeBasic with a direct call breaks LF customization
> 
> Remi,
> 
> Recompilation is usually expensive and if the call site hasn't been
> warmed up enough to be inlined, it is a clue it will likely never be.
> 
> Actually, this bug is really a corner case. LF customization matters in
> cases when there's no inlining possible (e.g., MH::invokeExact() on a
> receiver which is not a compile-time constant). Instead of jumping
> between numerous tiny nmethods for every LF in the chain, it allows to
> generate a single nmethod for the whole chain. There's no need in
> customization when inlining happens and customization doesn't help
> inlining as well.
> 
> In GWT case, uncommon trap is generated for non-taken branch (so causes
> a recompilation if it is ever taken), but a cold branch (<30
> invocations) is compiled into a call. It could be beneficial to
> recompile the branch if it becomes really hot, but it happens rarely and
> requires additional profiling.
> 
> I vaguely remember I experimented with that, but it severely affected
> warmup and didn't give any peak performance improvements (usually, on
> the contrarty).
> 
> Best regards,
> Vladimir Ivanov
> 
> On 2/4/16 8:13 PM, Remi Forax wrote:
> > Hi Vladimir,
> > Perhaps a stupid question, but why LF customization doesn't trigger a
> > recompilation if invokeBasic is already compiled into a direct call ?
> >
> > regards,
> > Rémi
> >
> > ----- Mail original -----
> >> De: "Vladimir Ivanov" <vladimir.x.ivanov at oracle.com>
> >> À: "hotspot compiler" <hotspot-compiler-dev at openjdk.java.net>
> >> Envoyé: Jeudi 4 Février 2016 17:32:33
> >> Objet: Re: [9] RFR (S): 8148994: Replacing MH::invokeBasic with a direct
> >> 	call breaks LF customization
> >>
> >> FYI I was asked to keep the tests intact until Jigsaw is integrated.
> >>
> >> Removed test changes from the webrev (updated in place) and filed
> >> JDK-8149040 [1] to clean up the tests later.
> >>
> >> Best regards,
> >> Vladimir Ivanov
> >>
> >> [1] https://bugs.openjdk.java.net/browse/JDK-8149040
> >>
> >> On 2/4/16 6:51 PM, Vladimir Ivanov 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"
> >>
>