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

[Vladimir Kozlov]

Okay.

Vladimir

On 2/26/14 6:46 AM, Nils Eliasson wrote:
> You are right.
>
> New rev:
> http://cr.openjdk.java.net/~neliasso/8007270/webrev.07/
>
> Thanks,
> //N
>
>
>
> On 2014-02-26 15:04, Igor Ignatyev wrote:
>> 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
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>
>>>
>