RFR: 8234863: Increase default value of MaxInlineLevel

[Doerr, Martin]

Hi Claes,

I certainly like to have the improvement for C2.
But I can't investigate impact on C1 so quickly. It may be true that increasing inlining depth is not as problematic as increasing MaxInlineSize for C1.
So wrt. this issue I abstain.

To make sure what we discussed won't get forgotten, I've created
https://bugs.openjdk.java.net/browse/JDK-8235673

Feel free to edit or comment.

Best regards,
Martin


> -----Original Message-----
> From: Claes Redestad <claes.redestad at oracle.com>
> Sent: Montag, 9. Dezember 2019 13:44
> To: Doerr, Martin <martin.doerr at sap.com>; Vladimir Kozlov
> <vladimir.kozlov at oracle.com>; hotspot compiler <hotspot-compiler-
> dev at openjdk.java.net>
> Subject: Re: RFR: 8234863: Increase default value of MaxInlineLevel
> 
> Hi,
> 
> Nils raised this issue in the bug, and while I think it's a fair point I
> think it's orthogonal to whether or not we can/should tune the default
> here and now. I think the data we have speaks in favor of doing this
> tuning for JDK 14, and then re-evaluate with more real-world data for
> JDK 15, possibly dialing back the C1 defaults.
> 
> When implementing such flags we can also evaluate if C1 should be even
> more conservative than it is today. It's also worth thinking about
> whether or not we should introduce different settings for C1 level 1, 2
> and 3..
> 
> /Claes
> 
> On 2019-12-09 12:16, Doerr, Martin wrote:
> > Hi,
> >
> > I think tuning inlining makes sense for C2.
> >
> > The problem I see is that C1 uses the same inlining flags.
> > C1 doesn't have the concept of uncommon traps so it compiles all paths.
> > That's why I think this change is only good for C2.
> >
> > So I suggest separating C1 inlining flags before tuning them for C2 like in the
> example below (note that new flags require CSR).
> > Feedback for this idea is welcome.
> >
> > Best regards,
> > Martin
> >
> >
> >
> > diff -r 45fceff98bb5 src/hotspot/share/c1/c1_globals.hpp
> > --- a/src/hotspot/share/c1/c1_globals.hpp       Mon Dec 09 10:26:41 2019
> +0100
> > +++ b/src/hotspot/share/c1/c1_globals.hpp       Mon Dec 09 12:08:37 2019
> +0100
> > @@ -174,6 +174,12 @@
> >     develop_pd(bool, RoundFPResults,                                          \
> >             "Indicates whether rounding is needed for floating point results")\
> >                                                                               \
> > +  product(intx, C1MaxInlineSize, 35,                                        \
> > +          "The maximum bytecode size of a method to be inlined by C1")      \
> > +  product(intx, C1MaxInlineLevel, 9,                                        \
> > +          "The maximum number of nested calls that are inlined by C1")      \
> > +  product(intx, C1MaxRecursiveInlineLevel, 1,                               \
> > +          "maximum number of nested recursive calls that are inlined")      \
> >     develop(intx, NestedInliningSizeRatio, 90,                                \
> >             "Percentage of prev. allowed inline size in recursive inlining")  \
> >             range(0, 100)                                                     \
> >
> >
> >
> >
> >> -----Original Message-----
> >> From: hotspot-compiler-dev <hotspot-compiler-dev-
> >> bounces at openjdk.java.net> On Behalf Of Claes Redestad
> >> Sent: Montag, 9. Dezember 2019 11:15
> >> To: Vladimir Kozlov <vladimir.kozlov at oracle.com>; hotspot compiler
> >> <hotspot-compiler-dev at openjdk.java.net>
> >> Subject: Re: RFR: 8234863: Increase default value of MaxInlineLevel
> >>
> >> Thanks for the review!
> >>
> >> /Claes
> >>
> >> On 2019-12-09 01:52, Vladimir Kozlov wrote:
> >>> Nice finding! Good.
> >>>
> >>> Thanks,
> >>> Vladimir
> >>>
> >>> On 12/8/19 1:44 PM, Claes Redestad wrote:
> >>>> Hi,
> >>>>
> >>>> increasing MaxInlineLevel can substantially improve performance in
> some
> >>>> benchmarks[1], and has been reported to help applications
> implemented
> >> in
> >>>> scala in particular.
> >>>>
> >>>> There is always some risk of regressions when tweaking the default
> >>>> inlining settings. I've done a number of experiments to ascertain that
> >>>> the effect of increasing this on a wide array of benchmarks. With 15 all
> >>>> benchmarks tested are show either neutral or positive results, with no
> >>>> observed regression w.r.t. compilation speed or code cache usage.
> >>>>
> >>>> Webrev: http://cr.openjdk.java.net/~redestad/8234863/open.00/
> >>>>
> >>>> Thanks!
> >>>>
> >>>> /Claes
> >>>>
> >>>> [1] One http://renaissance.dev sub-benchmark improve by almost 3x
> >> with
> >>>> an increase from 9 to 15.