RFR(S) 8007270: Make IsMethodCompilable test work with tiered

[Igor Ignatyev]

Nils,
I think it would be better to have
> if (!Platform.isServer())
instead of > +        // Only c2 compilations can be disabled through 
PerMethodRecompilationCutoff
> +        if (com.oracle.java.testlibrary.Platform.isClient()) {
> +            return;
> +        }
since we have 'Minimal VM' which also has only C1.

Igor

On 02/26/2014 03:14 PM, Nils Eliasson wrote:
> Sorry, now nice and tidy. One curly brace still looks to have the wrong
> indentation in the udiff. It has the right indentation in the changeset
> and in the other diffs (sdiff line 102).
>
> http://cr.openjdk.java.net/~neliasso/8007270/webrev.06/
>
> Thanks,
> //Nils
>
> On 2014-02-17 19:46, Vladimir Kozlov wrote:
>> On 2/17/14 1:19 AM, Nils Eliasson wrote:
>>> New:
>>>
>>> http://cr.openjdk.java.net/~neliasso/8007270/webrev.05/
>>>
>>> Answers inline.
>>>
>>> On 2014-02-12 22:41, Vladimir Kozlov wrote:
>>>> Indention is slightly off, otherwise new code is good. Can you shift
>>>> code a little?:
>>>>
>>>> +        for (long attempts = 0, successes = 0;
>>>> +             (successes < PER_METHOD_RECOMPILATION_CUTOFF) &&
>>>> +             (attempts  < PER_METHOD_RECOMPILATION_CUTOFF*2) &&
>>>> +             isCompilable(COMP_LEVEL_FULL_OPTIMIZATION);
>>>> +             attempts++) {
>>>> +          if (compileAndDeoptimize() ==
>>>> COMP_LEVEL_FULL_OPTIMIZATION) {
>>>> +              successes++;
>>>>            }
>>>>         }
>>
>> The indent is not fixed.
>>
>>>>
>>>> Do we need to assert (throw exception) after the loop when
>>>> (successes != PER_METHOD_RECOMPILATION_CUTOFF)?
>>>
>>> No, that will be caught be the other tests. First that it is still
>>> compilable, and then after one more compilation and
>>> deoptimization, that it is no longer compilable.
>>
>> Okay.
>>
>> Thanks,
>> Vladimir
>>
>>>
>>>> You left debugging output code in compileAndDeoptimize(). Do you
>>>> want to keep it?
>>>
>>> It was supposed to be removed.
>>>
>>> Thanks,
>>> Nils
>>>
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 2/12/14 1:46 AM, Nils Eliasson wrote:
>>>>> I rewrote the code. It should be much cleared now.
>>>>>
>>>>> http://cr.openjdk.java.net/~neliasso/8007270/webrev.04/
>>>>>
>>>>> Thanks,
>>>>> //N
>>>>>
>>>>>
>>>>> On 2014-02-11 23:10, Vladimir Kozlov wrote:
>>>>>> Sorry, I missed that. That code is very messy. I think you should
>>>>>> decrement 'i' instead and change comment:
>>>>>>
>>>>>> +        long cutoff = PER_METHOD_RECOMPILATION_CUTOFF;
>>>>>> +        // deoptimize 'PerMethodRecompilationCutoff' times
>>>>>> +        for (long i = 0L; (i < cutoff) &&
>>>>>> isCompilable(COMP_LEVEL_FULL_OPTIMIZATION); ++i) {
>>>>>> +          int level = compileAndDeoptimize();
>>>>>> +          if (level < COMP_LEVEL_FULL_OPTIMIZATION) {
>>>>>> +            // Add one iteration if compiled with a lower tier
>>>>>> +            --i;
>>>>>>            }
>>>>>>          }
>>>>>>
>>>>>> Thanks,
>>>>>> Vladimir
>>>>>>
>>>>>> On 2/11/14 1:34 PM, Nils Eliasson wrote:
>>>>>>> cutoff may increase if the method was compiled for a lower level
>>>>>>> and we
>>>>>>> want another attempt for full opt, the second check is to limit the
>>>>>>> amount of attempts.
>>>>>>>
>>>>>>> //N
>>>>>>>
>>>>>>> On 2014-02-11 20:06, Vladimir Kozlov wrote:
>>>>>>>> (i <c) should be always true since (i < cutoff):
>>>>>>>>
>>>>>>>> +        for (long i = 0L; (i < cutoff) &&
>>>>>>>> +              (i < PER_METHOD_RECOMPILATION_CUTOFF*2) &&
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Vladimir
>>>>>>>>
>>>>>>>> On 2/11/14 5:15 AM, Nils Eliasson wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> Thanks for your contribution Igor.
>>>>>>>>>
>>>>>>>>> After some additional testing I noticed some inconsistencies and
>>>>>>>>> filed https://bugs.openjdk.java.net/browse/JDK-8034188
>>>>>>>>>
>>>>>>>>> This test now runs faster, and targets c2-compiles only, in server
>>>>>>>>> and with tiered.
>>>>>>>>>
>>>>>>>>> Webrev: http://cr.openjdk.java.net/~neliasso/8007270/webrev.02/
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> //Nils
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2014-01-29 11:11, Igor Ignatyev wrote:
>>>>>>>>>> Hi Nils,
>>>>>>>>>>
>>>>>>>>>> you can skip running on client by yourself, see attached diff.
>>>>>>>>>> Igor
>>>>>>>>>>
>>>>>>>>>> On 01/28/2014 07:02 PM, Nils Eliasson wrote:
>>>>>>>>>>> Hi all,
>>>>>>>>>>>
>>>>>>>>>>> I need a review for this change.
>>>>>>>>>>>
>>>>>>>>>>> http://cr.openjdk.java.net/~neliasso/8007270/webrev.01/
>>>>>>>>>>>
>>>>>>>>>>> This test was disabled since it didn't work very well with
>>>>>>>>>>> tiered
>>>>>>>>>>> (or
>>>>>>>>>>> client). It tests the PerMethodRecompilationCutoff that was
>>>>>>>>>>> introduced
>>>>>>>>>>> to disable c2-compilations of a method when it has been
>>>>>>>>>>> deoptimized
>>>>>>>>>>> too
>>>>>>>>>>> many times.  The bug report suggested we should disable c1
>>>>>>>>>>> compilations
>>>>>>>>>>> as well but I don't think that was the intent of the cutoff
>>>>>>>>>>> feature.
>>>>>>>>>>>
>>>>>>>>>>> I have changed the following in the test
>>>>>>>>>>> * skip test when running client only (not supported by jtreg
>>>>>>>>>>> at the
>>>>>>>>>>> moment)
>>>>>>>>>>> * check what compilation level was used when compiling so
>>>>>>>>>>> that it
>>>>>>>>>>> can
>>>>>>>>>>> keep track of the number of c2 compiles (and deopts)
>>>>>>>>>>> correctly in
>>>>>>>>>>> tiered
>>>>>>>>>>> mode
>>>>>>>>>>> * compile and deopt up to the cutoff limit only once
>>>>>>>>>>> * added PerMethodRecompilationCutoff=4 flag to commandline to
>>>>>>>>>>> reduce
>>>>>>>>>>> wasted time in test (default 400)
>>>>>>>>>>>
>>>>>>>>>>> Now the test works and the running time has been reduced to
>>>>>>>>>>> seconds
>>>>>>>>>>> instead of minutes.
>>>>>>>>>>>
>>>>>>>>>>> Kind regards,
>>>>>>>>>>> Nils Eliasson
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>
>>>
>