RFR(S): 8235673: [C1, C2] Split inlining control flags
Vladimir Kozlov
vladimir.kozlov at oracle.com
Fri May 8 19:42:37 UTC 2020
Hi Martin
On 5/8/20 5:56 AM, Doerr, Martin wrote:
> Hi Vladimir,
>
> thanks a lot for looking at this, for finding the test issues and for reviewing the CSR.
>
> For me, C2 is a fundamental part of the JVM. I would usually never build without it ��
> (Except if we want to use C1 + GraalVM compiler only.)
Yes it is one of cases.
> But your right, --with-jvm-variants=client configuration should still be supported.
Yes.
>
> We can fix it by making the flags as obsolete if C2 is not included:
> diff -r 5f5ed86d7883 src/hotspot/share/runtime/arguments.cpp
> --- a/src/hotspot/share/runtime/arguments.cpp Fri May 08 11:14:28 2020 +0200
> +++ b/src/hotspot/share/runtime/arguments.cpp Fri May 08 14:41:14 2020 +0200
> @@ -562,6 +562,16 @@
> { "dup option", JDK_Version::jdk(9), JDK_Version::undefined(), JDK_Version::undefined() },
> #endif
>
> +#ifndef COMPILER2
> + // These flags were generally available, but are C2 only, now.
> + { "MaxInlineLevel", JDK_Version::undefined(), JDK_Version::jdk(15), JDK_Version::undefined() },
> + { "MaxRecursiveInlineLevel", JDK_Version::undefined(), JDK_Version::jdk(15), JDK_Version::undefined() },
> + { "InlineSmallCode", JDK_Version::undefined(), JDK_Version::jdk(15), JDK_Version::undefined() },
> + { "MaxInlineSize", JDK_Version::undefined(), JDK_Version::jdk(15), JDK_Version::undefined() },
> + { "FreqInlineSize", JDK_Version::undefined(), JDK_Version::jdk(15), JDK_Version::undefined() },
> + { "MaxTrivialSize", JDK_Version::undefined(), JDK_Version::jdk(15), JDK_Version::undefined() },
> +#endif
> +
> { NULL, JDK_Version(0), JDK_Version(0) }
> };
Right. I think you should do full process for these product flags deprecation with obsoleting in JDK 16 for VM builds
which do not include C2. You need update your CSR - add information about this and above code change. Example:
https://bugs.openjdk.java.net/browse/JDK-8238840
>
> This makes the VM accept the flags with warning:
> jdk/bin/java -XX:MaxInlineLevel=9 -version
> OpenJDK 64-Bit Client VM warning: Ignoring option MaxInlineLevel; support was removed in 15.0
>
> If we do it this way, the only test which I think should get fixed is ReservedStackTest.
> I think it should be sufficient to add -XX:C1MaxInlineLevel=2 in order to preserve the inlining behavior.
>
> (TestStringIntrinsics2: C1 doesn't have String intrinsics anymore. compiler/c2 tests: Also written to test C2 specific things.)
>
> What do you think?
I would suggest to fix tests anyway (there are only few) because new warning output could be unexpected.
And it will be future-proof when warning will be converted into error (if/when C2 goes away).
Thanks,
Vladimir
>
> Best regards,
> Martin
>
>
>> -----Original Message-----
>> From: hotspot-compiler-dev <hotspot-compiler-dev-
>> bounces at openjdk.java.net> On Behalf Of Vladimir Kozlov
>> Sent: Donnerstag, 7. Mai 2020 19:11
>> To: hotspot-compiler-dev at openjdk.java.net
>> Subject: Re: RFR(S): 8235673: [C1, C2] Split inlining control flags
>>
>> I would suggest to build VM without C2 and run tests.
>>
>> I grepped tests with these flags I found next tests where we need to fix
>> test's command (add
>> -XX:+IgnoreUnrecognizedVMOptions) or add @requires
>> vm.compiler2.enabled or duplicate test for C1 with corresponding C1
>> flags (by ussing additional @test block).
>>
>> runtime/ReservedStack/ReservedStackTest.java
>> compiler/intrinsics/string/TestStringIntrinsics2.java
>> compiler/c2/Test6792161.java
>> compiler/c2/Test5091921.java
>>
>> And there is issue with compiler/compilercontrol tests which use
>> InlineSmallCode and I am not sure how to handle:
>>
>> http://hg.openjdk.java.net/jdk/jdk/file/55e9cb6b23ec/test/hotspot/jtreg/c
>> ompiler/compilercontrol/share/scenario/Command.java#l36
>>
>> Thanks,
>> Vladimir
>>
>> On 5/4/20 9:04 AM, 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);
>>>>> }
>>>>> }
>>>>>
>>>
More information about the hotspot-compiler-dev
mailing list