RFR(S): 8008211 : Some of WB tests on compiler fails

[Igor Ignatyev]

Vladimir,

Thank you for review.
I have updated my patch, now 'Math.max(COMPILE_THRESHOLD, 150000)' is 
used as threshold.

http://cr.openjdk.java.net/~iignatyev/8008211/webrev.01/

Best regards,
Igor Ignatyev

On 03/11/2013 11:34 PM, Vladimir Kozlov wrote:
> I thought I replied to this.
>
>  > You didn't comment my previous letter. Probably it got lost in a pile
> of letters.
>  > Could you please comment it?
>  >
>  > short version of letter:
>  >    1. moving WHITE_BOX.setDontInlineMethod(METHOD, true) helps
>
> I agree with this.
>
>  >    2. value of 'CompileThreshold' are used as for-loop's limit in
>  > CompilerWhiteBoxTest and it's insufficient for compilation in tiered.
>  > And I could not find a way to get tiered threshold to WB, since we have
>  > only predicate to test + 'Tier0InvokeNotifyFreqLog' determines how often
>  > predicate will be checked.
>  > So I must to use the knowledge of current tiered threshold
> implementation.
>
> You can simple do "count = max(COMPILE_THRESHOLD, 100000) + 10000".
> Note, you need more then 10000.
>
> I am strongly against putting any VM knowledge into tests.
>
> Vladimir
>
> On 2/20/13 8:03 AM, Igor Ignatyev wrote:
>> On 02/20/2013 12:41 AM, Vladimir Kozlov wrote:
>>> On 2/19/13 12:12 PM, Igor Ignatyev wrote:
>>>> There are two different reasons for this failure:
>>>>
>>>>     1. with '-Xcomp':
>>>> CompilerWhiteBoxTest#method() is inlined into #compile() on first
>>>> invocation of #test(). so 'WHITE_BOX.setDontInlineMethod(METHOD, true)'
>>>> does nothing; #method() isn't compiled;
>>>
>>> You need to make method() more complex to avoid it. Currently it is
>>> treated as an "accessor" method and it is inlinined unconditionally. For
>>> example, pass argument and return increment (x+42).
>>> I don't see how moving WHITE_BOX.setDontInlineMethod() helps.
>>
>> moving WHITE_BOX.setDontInlineMethod(METHOD, true) helps because it
>> disallow inlining of METHOD.
>>
>> cutting of PrintInlining/PrintCompilation output before moving:
>>
>>     3716  615    b  3       DeoptimizeAllTest::test (35 bytes)
>>                                @ 7
>> sun.hotspot.WhiteBox::setDontInlineMethod (0 bytes)   native method
>>                                @ 12   CompilerWhiteBoxTest::compile (26
>> bytes)
>>                                  @ 13   CompilerWhiteBoxTest::method (3
>> bytes)
>>                                @ 19 CompilerWhiteBoxTest::checkCompiled
>> (122 bytes)   callee is too large
>>                                @ 25 sun.hotspot.WhiteBox::deoptimizeAll
>> (0 bytes)   native method
>>                                @ 31
>> CompilerWhiteBoxTest::checkNotCompiled (75 bytes)   callee is too large
>>
>> and after moving:
>>     3983  616    b  3       DeoptimizeAllTest::test (24 bytes)
>>                                @ 1   CompilerWhiteBoxTest::compile (26
>> bytes)
>>                                  @ 13   CompilerWhiteBoxTest::method (3
>> bytes)   don't inline by annotation
>>                                @ 8   CompilerWhiteBoxTest::checkCompiled
>> (122 bytes)   callee is too large
>>                                @ 14 sun.hotspot.WhiteBox::deoptimizeAll
>> (0 bytes)   native method
>>
>>>>     2. with '-XX:+TieredCompilation' and certain relation between
>>>> 'CompileThreshold' and 'Tier*' values (e.g. -XX:CompileThreshold=100
>>>> and
>>>> default Tier* values):
>>>> since 'CompileThreshold' doesn't use in Tiered as threshold, but
>>>> uses in
>>>> tests we have situation in which CompilerWhiteBoxTest#compile() invokes
>>>> #method() insufficient time for compilation;
>>>
>>>
>>> With Tiered the default threshold for first tier (C1) is 100. Yes, for
>>> C2 it is >10000 and depends on other staff. But COMPILE_THRESHOLD=10000
>>> should be enough for tier one compilation. So I don't understand what do
>>> you mean by "invokes insufficient time for compilation".
>>
>> in current version of tests value of 'CompileThreshold' is used as limit
>> in for-loop. In nightly it equals '100' and it isn't enough even for
>> tier one, cuz with default values of Tier* flags we need to invoke
>> method 256 times.
>> in attachment program that illustrates it:
>>
>> $ java -XX:+PrintCompilation -XX:+TieredCompilation
>> -XX:CompileThreshold=100 -XX:+PrintTieredEvents CompileThresholdTest 100
>> | grep CompileThresholdTest
>>
>> $ java -XX:+PrintCompilation -XX:+TieredCompilation
>> -XX:CompileThreshold=100 -XX:+PrintTieredEvents CompileThresholdTest 256
>> | grep CompileThresholdTest
>> 0.049705: [call level: 0 [CompileThresholdTest.m1()I] @-1 queues: 0,0
>> rate: n/a k: 1.00,1.00 total: 128,0 mdo: 0(0),0(0) max levels: 0,0
>> compilable: c1, c2, osr status: idle]
>> 0.049785: [call level: 0 [CompileThresholdTest.m1()I] @-1 queues: 0,0
>> rate: n/a k: 1.00,1.00 total: 256,0 mdo: 0(0),0(0) max levels: 0,0
>> compilable: c1, c2, osr status: idle]
>> 0.049812: [compile level: 3 [CompileThresholdTest.m1()I] @-1 queues: 0,0
>> rate: n/a k: 1.00,1.00]
>>       49    8       3       CompileThresholdTest::m1 (3 bytes)
>>
>>
>>> And I really don't like next code:
>>>
>>> !         int count = getCompileThreshold();
>>> !         int i = 0;
>>> !         do {
>>> !             for (; i < count; ++i) {
>>>                    result += method();
>>>                }
>>> +             count = getCompileThreshold();
>>> +         } while (i < count);
>>>
>>> You are using in getCompileThreshold() the knowledge of current tiered
>>> threashold implementation.  But it could be changed in a future.
>>>
>>
>> I would like to pull out getting of tiered threshold to WB, but I could
>> not find a way to do this, since we have only predicate to test +
>> 'Tier0InvokeNotifyFreqLog' determines how often predicate will be
>> checked.
>>
>>> Vladimir
>>>>
>>>> Best regards,
>>>> Igor Ignatyev
>>>>
>>>> On 02/19/2013 10:49 PM, Vladimir Kozlov wrote:
>>>>> Igor,
>>>>>
>>>>> Can you give mode details about the failure?
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> On 2/19/13 9:37 AM, Igor Ignatyev wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> Please review the patch.
>>>>>>
>>>>>> 1. calls of WB.setDontInlineMethod() were moved into main()
>>>>>> 2. added estimation of threshold for tiered compilation
>>>>>>
>>>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8008211/webrev.00/