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

[Vladimir Kozlov]

I reviewed CSR.

For Oracle's sponsor (Nils?): we need to create doc subtask to add these changes to release notes and update java man page.

Thanks,
Vladimir

On 5/6/20 3:19 AM, Doerr, Martin wrote:
> 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);
>>>>>        }
>>>>> }
>>>>>
>