RFR (XS) 8204966: [TESTBUG] hotspot/test/compiler/whitebox/IsMethodCompilableTest.java test fails with -XX:CompileThreshold=1

[Vladimir Kozlov]

On 8/4/18 8:56 PM, David Buck wrote:
> Hi Vlad!
> 
> Thanks for looking at this change!
> 
>  > You can hit some strange behavior if COMPILE_THRESHOLD very different
>  > from BACKEDGE_THRESHOLD.
> 
> I'm not sure I understand. Do you mean that the these tests behave strangely? These values are only impacting the number 
> of invocations / loop iterations these tests do to try and force HotSpot to compile something. As long as we do enough 
> invocations/iterations, I can't imagine any weird behavior from the tests themselves.

You are right. I was overthinking that it reset VM flags values which may affect compilation heuristic.
Looking on tests and BACKEDGE_THRESHOLD indeed is used for loop limits.

Sorry for noise. Your original changes are good.

Thanks,
Vladimir

> 
>  > I would suggest set max for COMPILE_THRESHOLD instead and leave
>  > BACKEDGE_THRESHOLD calculation the same.
> 
> You mean set a min (not max), right? The problem is the test was not doing enough iterations, not too many. Anyway, in 
> this code COMPILE_THRESHOLD only seems to be used to represent what the JVM is set for. (i.e. it is only used to (help) 
> calculate THRESHOLD and BACKEDGE_THRESHOLD).
> 
> We *could* do something like this:
> 
> ===
>       static {
>           if (TIERED_COMPILATION) {
>               BACKEDGE_THRESHOLD = THRESHOLD = 150000;
>           } else {
> -             THRESHOLD = COMPILE_THRESHOLD;
> +             THRESHOLD = Math.max(10000, COMPILE_THRESHOLD);
> -             BACKEDGE_THRESHOLD = COMPILE_THRESHOLD *
> - Long.parseLong(getVMOption("OnStackReplacePercentage"));
> +             BACKEDGE_THRESHOLD = THRESHOLD *
> + Long.parseLong(getVMOption("OnStackReplacePercentage"));
>           }
>       }
> 
> ===
> 
> But I'm not sure I see how the above is necessarily an improvement unless we suspect the test cases are also not doing 
> enough invocations to trigger normal JIT as in addition to the OSR failure in jdk-8204966. If that is the case, maybe we 
> should just scrap the entire if block and just go with something like this:
> 
> ===
>       static {
>           BACKEDGE_THRESHOLD = THRESHOLD = 150000;
>       }
> ===
> 
> 
> Cheers,
> -Buck
> 
> 
> On 2018/08/05 4:04, Vladimir Kozlov wrote:
>> You can hit some strange behavior if COMPILE_THRESHOLD very different from BACKEDGE_THRESHOLD.
>> I would suggest set max for COMPILE_THRESHOLD instead and leave BACKEDGE_THRESHOLD calculation the same.
>>
>> Thanks,
>> Vladimir
>>
>> On 8/4/18 4:26 AM, David Buck wrote:
>>> Hi!
>>>
>>> May I please get a review of this trivial fix to a testcase.
>>>
>>> bug report:
>>> https://bugs.openjdk.java.net/browse/JDK-8204966
>>>
>>> webrev
>>> http://cr.openjdk.java.net/~dbuck/8204966.0/
>>>
>>> Cheers,
>>> -Buck