RFR (XXS): 8023461: Thread holding lock at safepoint that vm can block on: MethodCompileQueue_lock

[Vladimir Kozlov]

Nice.

Thanks,
Vladimir

On 5/13/14 4:23 PM, Vladimir Ivanov wrote:
> Vladimir, thanks!
>
> On 5/14/14 2:58 AM, Vladimir Kozlov wrote:
>> On 5/13/14 2:34 PM, Vladimir Ivanov wrote:
>>>> Relaxing the check is not the fix. ATP::select_task() should be fixed -
>>>> exclude safepoints in it.
>>> OK. I planned to fix select_task separately (filed RFE to track that),
>>> since people complains about intermittent failures during testing due to
>>> this bug and I have some concerns with the fix I had. But I'll try to
>>> fix it as part of this change then.
>>>
>>>> The update_rate() could be not called if there are no MethodCounters
>>>> (other places may need to be fixed for that). Actually I don't
>>>> understand how we have compilation request for a method without
>>>> MethodCounters. Counters should be created before that.
>>> It's not always the case. MDO can be created first (e.g. compilation can
>>> force MDO creation [1]) and there's no need in MethodCounters (see
>>> InterpreterGenerator::generate_counter_incr [2]).
>>
>> I think this is contradiction in tiered implementation. We don't
>> allocate MethodCounters for TieredCompilation as you pointed but tiered
>> code uses them.
> I agree. Filed JDK-8043061 to track that.
>
>>>
>>>> And removing stale tasks could be done in an other place, outside MCQ
>>>> lock.
>>> I looked again closely at the code and I think I found a sweet spot.
>>>
>>> Updated webrev:
>>> http://cr.openjdk.java.net/~vlivanov/8023461/webrev.03/
>>
>> Very good. Add small comment explaining your trick to avoid conflicts
>> with other compiler threads after lock is released:
>>
>> +     CompileTask* head = _first_stale;
>> +     _first_stale = NULL;
> Updated webrev in place with a comment:
> http://cr.openjdk.java.net/~vlivanov/8023461/webrev.03/
>
> Best regards,
> Vladimir Ivanov
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8043061