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

[Nils Eliasson]

On 2014-11-13 16:36, Albert Noll wrote:
> Hi Nils,
>
> On 11/13/2014 03:58 PM, Albert Noll wrote:
>> Hi Nils,
>>
>> CompileQueue::lock() always returns MethodCompileQueue_lock ( see 
>> init_compiler_sweeper_threads() ). It seems that your changes don't 
>> change existing behavior, or am I missing something? Note that we 
>> have 2 compilation queues, but only 1 lock.
>>
> It seems that I missed the most important parts of you change indeed. 
> Is it right that
> Mode evaluation_mode() const { return _no_safepoint; }
>
> makes the VM operation not execute at a safepoint? I have a question 
> nevertheless. Why did you choose to return '_no_safepoint'  and not 
> '_concurrent'?

The diagnostic commands run in their own thread, and that thread still 
has to wait for the output. Running the command cuncurrently would 
require some care with how the operation is allocated so that it can be 
passed to the VM_thread and be enqueued there.

Regards,
Nils

>
> Best,
> Albert
>
>> On 11/13/2014 03:11 PM, 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.
>> What element of surprise are you talking about? Personally, I prefer 
>> the current design, i.e., getting the lock by calling a function 
>> instead of referencing a global variable directly. If we ever decide 
>> to go for a 2 compile queue locks (I think that makes sense since we 
>> have two compilation queues), we would have to introduce the 
>> functions again.
>>
>> Best,
>> Albert
>>
>>
>>> webrev: http://cr.openjdk.java.net/~neliasso/8061256/webrev.02/
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8061256
>>>
>>> Thanks,
>>> Nils Eliasson
>>>
>>
>

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