RFR(S): 8235673: [C1, C2] Split inlining control flags

[Doerr, Martin]

Hi Nils,

I've created CSR
https://bugs.openjdk.java.net/browse/JDK-8244507
and set it to "Proposed".

Feel free to modify it if needed. I will need reviewers for it, too.

Best regards,
Martin


> -----Original Message-----
> From: Nils Eliasson <nils.eliasson at oracle.com>
> Sent: Dienstag, 5. Mai 2020 11:54
> To: Doerr, Martin <martin.doerr at sap.com>; hotspot-compiler-
> dev at openjdk.java.net
> Subject: Re: RFR(S): 8235673: [C1, C2] Split inlining control flags
> 
> Hi Martin,
> 
> I think it looks good.
> 
> Please go ahead!
> 
> Best regards,
> Nils
> 
> 
> On 2020-05-04 18:04, Doerr, Martin wrote:
> > Hi Nils,
> >
> > thank you for looking at this and sorry for the late reply.
> >
> > I've added MaxTrivialSize and also updated the issue accordingly. Makes
> sense.
> > Do you have more flags in mind?
> >
> > Moving the flags which are only used by C2 into c2_globals definitely makes
> sense.
> >
> > Done in webrev.01:
> > http://cr.openjdk.java.net/~mdoerr/8235673_C1_inlining/webrev.01/
> >
> > Please take a look and let me know when my proposal is ready for a CSR.
> >
> > Best regards,
> > Martin
> >
> >
> >> -----Original Message-----
> >> From: hotspot-compiler-dev <hotspot-compiler-dev-
> >> bounces at openjdk.java.net> On Behalf Of Nils Eliasson
> >> Sent: Dienstag, 28. April 2020 18:29
> >> To: hotspot-compiler-dev at openjdk.java.net
> >> Subject: Re: RFR(S): 8235673: [C1, C2] Split inlining control flags
> >>
> >> Hi,
> >>
> >> Thanks for addressing this! This has been an annoyance for a long time.
> >>
> >> Have you though about including other flags - like MaxTrivialSize?
> >> MaxInlineSize is tested against it.
> >>
> >> Also - you should move the flags that are now c2-only to c2_globals.hpp.
> >>
> >> Best regards,
> >> Nils Eliasson
> >>
> >> On 2020-04-27 15:06, Doerr, Martin wrote:
> >>> Hi,
> >>>
> >>> while tuning inlining parameters for C2 compiler with JDK-8234863 we
> had
> >> discussed impact on C1.
> >>> I still think it's bad to share them between both compilers. We may want
> to
> >> do further C2 tuning without negative impact on C1 in the future.
> >>> C1 has issues with substantial inlining because of the lack of uncommon
> >> traps. When C1 inlines a lot, stack frames may get large and code cache
> space
> >> may get wasted for cold or even never executed code. The situation gets
> >> worse when many patching stubs get used for such code.
> >>> I had opened the following issue:
> >>> https://bugs.openjdk.java.net/browse/JDK-8235673
> >>>
> >>> And my initial proposal is here:
> >>> http://cr.openjdk.java.net/~mdoerr/8235673_C1_inlining/webrev.00/
> >>>
> >>>
> >>> Part of my proposal is to add an additional flag which I called
> >> C1InlineStackLimit to reduce stack utilization for C1 methods.
> >>> I have a simple example which shows wasted stack space (java example
> >> TestStack at the end).
> >>> It simply counts stack frames until a stack overflow occurs. With the
> current
> >> implementation, only 1283 frames fit on the stack because the never
> >> executed method bogus_test with local variables gets inlined.
> >>> Reduced C1InlineStackLimit avoids inlining of bogus_test and we get
> 2310
> >> frames until stack overflow. (I only used C1 for this example. Can be
> >> reproduced as shown below.)
> >>> I didn't notice any performance regression even with the aggressive
> setting
> >> of C1InlineStackLimit=5 with TieredCompilation.
> >>> I know that I'll need a CSR for this change, but I'd like to get feedback in
> >> general and feedback about the flag names before creating a CSR.
> >>> I'd also be glad about feedback regarding the performance impact.
> >>>
> >>> Best regards,
> >>> Martin
> >>>
> >>>
> >>>
> >>> Command line:
> >>> jdk/bin/java -XX:TieredStopAtLevel=1 -XX:C1InlineStackLimit=20 -
> >> XX:C1MaxRecursiveInlineLevel=0 -Xss256k -Xbatch -XX:+PrintInlining -
> >> XX:CompileCommand=compileonly,TestStack::triggerStackOverflow
> >> TestStack
> >>> CompileCommand: compileonly TestStack.triggerStackOverflow
> >>>                                 @ 8   TestStack::triggerStackOverflow (15 bytes)
> recursive
> >> inlining too deep
> >>>                                 @ 11   TestStack::bogus_test (33 bytes)   inline
> >>> caught java.lang.StackOverflowError
> >>> 1283 activations were on stack, sum = 0
> >>>
> >>> jdk/bin/java -XX:TieredStopAtLevel=1 -XX:C1InlineStackLimit=10 -
> >> XX:C1MaxRecursiveInlineLevel=0 -Xss256k -Xbatch -XX:+PrintInlining -
> >> XX:CompileCommand=compileonly,TestStack::triggerStackOverflow
> >> TestStack
> >>> CompileCommand: compileonly TestStack.triggerStackOverflow
> >>>                                 @ 8   TestStack::triggerStackOverflow (15 bytes)
> recursive
> >> inlining too deep
> >>>                                 @ 11   TestStack::bogus_test (33 bytes)   callee uses too
> >> much stack
> >>> caught java.lang.StackOverflowError
> >>> 2310 activations were on stack, sum = 0
> >>>
> >>>
> >>> TestStack.java:
> >>> public class TestStack {
> >>>
> >>>       static long cnt = 0,
> >>>                   sum = 0;
> >>>
> >>>       public static void bogus_test() {
> >>>           long c1 = 1, c2 = 2, c3 = 3, c4 = 4;
> >>>           sum += c1 + c2 + c3 + c4;
> >>>       }
> >>>
> >>>       public static void triggerStackOverflow() {
> >>>           cnt++;
> >>>           triggerStackOverflow();
> >>>           bogus_test();
> >>>       }
> >>>
> >>>
> >>>       public static void main(String args[]) {
> >>>           try {
> >>>               triggerStackOverflow();
> >>>           } catch (StackOverflowError e) {
> >>>               System.out.println("caught " + e);
> >>>           }
> >>>           System.out.println(cnt + " activations were on stack, sum = " +
> sum);
> >>>       }
> >>> }
> >>>