RFR(S): 8153013: BlockingCompilation test times out
Nils Eliasson
nils.eliasson at oracle.com
Thu Apr 14 12:43:06 UTC 2016
I moved the reasons to CompileTask.hpp and put it together with the
names list. Also changed the type from int to CompileReason as Igor
suggested.
It gets verbose in the method declarations in compileBroker and
sometimes I think CompileReason should be declared in CompileBroker
because it is mostly used there. On the other hand, CompileTask is the
keeper of the CompileReason so it makes sense too.
New webrev:
http://cr.openjdk.java.net/~neliasso/8153013/webrev.03/
Thanks!
Nils
On 2016-04-13 23:34, Vladimir Kozlov wrote:
> Very nice, I like it.
>
> One note. CompileReason (and its names) should be CompileTask class
> where it is recorded. Then CompileTask::can_become_stale() can be in
> header file so it is inlinined on all platforms.
>
> Thanks,
> Vladimir
>
> On 4/13/16 5:59 AM, Nils Eliasson wrote:
>> Hi,
>>
>> New webrev:
>> http://cr.openjdk.java.net/~neliasso/8153013/webrev.02/
>>
>> Summary
>> Introduced an enum CompileReason with members matching all the old
>> variants, and a table containing all the unchanged strings. I see the
>> possibility of removing/changing/simplifying some CompileReasons but
>> have choosen not to do so in this change.
>>
>> Only new logic is the CompileTask::can_become_stale() method.
>>
>> Testing:
>> Running Testset hotspot on all platforms and hotspot_all on one platform
>>
>> Regards,
>> Nils Eliawsson
>>
>> On 2016-04-12 18:55, Vladimir Kozlov wrote:
>>> On 4/12/16 6:30 AM, Nils Eliasson wrote:
>>>> Tasks get evicted from the compile_queue if their invocation counter
>>>> hasn't increased during TieredCompileTaskTimeout.
>>>> (AdvancedThresholdPolicy::is_stale(...)).
>>>>
>>>> I'll do a proper fix, it is the right thing to do and should be pretty
>>>> quick. I'll change the comment to an enum that represent who submitted
>>>> the compile, and add a table for the comments. This could be useful in
>>>> other settings to.
>>>
>>> Sounds good.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>>>
>>>> Regards,
>>>> Nils
>>>>
>>>> On 2016-04-08 19:09, Vladimir Kozlov wrote:
>>>>> What do you mean "stale"?
>>>>> I would prefer to see the real fix as you suggested to avoid removing
>>>>> WB comp tasks from queue. Adding timeout is not reliable.
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> On 4/8/16 5:27 AM, Nils Eliasson wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Please review this small fix of the BlockingCompilation test.
>>>>>>
>>>>>> Summary:
>>>>>> Add method enqueued for compilation with WB API may be removed from
>>>>>> the compile queue as stale.
>>>>>>
>>>>>> Solution:
>>>>>> Add -XX:TieredCompileTaskTimeout=60000 to make sure nothing gets
>>>>>> stale while the test is running. (Also added some extra
>>>>>> checks that may spare us from waiting until timeout for failing.)
>>>>>>
>>>>>> This is an workaround but we should consider fixing something
>>>>>> permanent for WB API compiles - like tagging the compile
>>>>>> task with info about the origin of the compile. The comment field
>>>>>> has
>>>>>> this information - but then it needs to be
>>>>>> converted to an enum.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8153013
>>>>>> Webrev: http://cr.openjdk.java.net/~neliasso/8153013/webrev.01/
>>>>>>
>>>>>> Best regards,
>>>>>> Nils Eliasson
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>
>>
More information about the hotspot-compiler-dev
mailing list