RFR(S): 8153013: BlockingCompilation test times out

[Christian Thalinger]

> On Apr 14, 2016, at 2: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

Don’t worry about this.

> 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.

Yes, that’s the right place.

> 
> New webrev:
> http://cr.openjdk.java.net/~neliasso/8153013/webrev.03/

+   bool         can_become_stale() const          {
+     return !_is_blocking && (_compile_reason < Reason_Whitebox);
+   }
I’m not a fan of implicit contracts just defined by comments.  This method doesn’t seem to be performance critical so I would suggest to use a switch-case.  An attribute on the enum would be much better but we all know this isn’t Java.

> 
> 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
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>> 
>>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160414/225d67b6/attachment.html>