RFR(S): 8153013: BlockingCompilation test times out

[Igor Veresov]

Looks good. Thanks!

igor

> On Apr 14, 2016, at 5:43 AM, Nils Eliasson <nils.eliasson at oracle.com> wrote:
> 
> 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
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>> 
>>> 
>