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

[Vladimir Ivanov]

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