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

[Albert Noll]

Hi Nils,

On 11/17/2014 12:52 PM, Nils Eliasson wrote:
> 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.
>
I have no strong opinion about this - I just think the code is fine as 
it is. If you want to go for it, that's ok for me.

Best,
Albert

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