RFR(S): 8061256: com/sun/management/DiagnosticCommandMBean/DcmdMBeanPermissionsTest.java timed out

[Nils Eliasson]

Hi Vladimir,

I just think the code is easier to read and browse this way. If we want 
separate queue locks in the future it is easy to add back.

If you and Albert don't want that part, I'll remove it.

Regards,
//Nils



On 2014-11-14 19:01, Vladimir Kozlov wrote:
> Hi Nils,
>
> Looks like you answered Albert's questions.
> I am not happy with refactoring lock() accessor for the same reason as 
> Albert said.
> But I understand why you did it and fine with that.
> We can investigate separate locks for queues but I don't think it is a 
> bottleneck currently. Also some parts of code relay on one lock.
>
> Thanks,
> Vladimir
>
> On 11/13/14 6:11 AM, Nils Eliasson wrote:
>> Hi,
>>
>> Please review this small change.
>>
>> 1) Fixing a deadlock in diagnostic command dcmd print_compile_queues 
>> - between the CompileQueue_lock and safepointing.
>> The CompileQueue_lock requires that we are not at a safepoint when 
>> taking it. Otherwise a java-thread that already holds
>> the lock will block when trying to get another lock. In this specific 
>> case the compiler threads are Java threads, they
>> take the CompileQueue_lock frequently and some time takes the 
>> CompileTaskAlloc_lock after that.
>>
>> Fixing this by making the diagnostic command not request a safepoint,
>>
>> 2) Refactoring away the lock() accessor in CompileQueue and stating 
>> the lock explicitly. This removes an element of
>> surprise and makes it easier understanding the code.
>>
>> webrev: http://cr.openjdk.java.net/~neliasso/8061256/webrev.02/
>> bug: https://bugs.openjdk.java.net/browse/JDK-8061256
>>
>> Thanks,
>> Nils Eliasson
>>