RFR(S) 8007270: Make IsMethodCompilable test work with tiered
Nils Eliasson
nils.eliasson at oracle.com
Wed Feb 26 03:14:08 PST 2014
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
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>
More information about the hotspot-compiler-dev
mailing list