RFR(S): 8058461: serviceability/dcmd/CodelistTest.java and serviceability/dcmd/CompilerQueueTest.java SIGSEGV

[Nils Eliasson]

Albert pointed out that there are actually only one compile queue lock 
share by the queues. That simplified the patch.

Next: http://cr.openjdk.java.net/~neliasso/8058461/webrev.12

Thanks,
//Nils

On 2014-09-18 12:41, Nils Eliasson wrote:
> Yes, good catch.
>
> Next: http://cr.openjdk.java.net/~neliasso/8058461/webrev.09/
>
> //Nils
>
> On 2014-09-18 12:21, Albert wrote:
>> Hi Nils,
>>
>>   785 void CompileBroker::print_compile_queues(outputStream* st) {
>>   786   _c1_compile_queue->print_with_lock(st);
>>   787   _c2_compile_queue->print_with_lock(st);
>>   788 }
>>
>>
>> can't it happen that _c1_compile_queue and/or _c2_compile_queue or NULL?
>>
>> Best,
>> Albert
>>
>>
>> On 09/18/2014 10:50 AM, Nils Eliasson wrote:
>>> Yes. Also good to have the print method lock-free so it can be 
>>> callled from a debugger without hassle. Adding a wrapper that takes 
>>> the compileQueue lock.
>>>
>>> New webrev: http://cr.openjdk.java.net/~neliasso/8058461/webrev.08/
>>>
>>> Thanks!
>>> //Nils
>>>
>>> On 2014-09-18 02:59, Vladimir Kozlov wrote:
>>>> CompileQueue::add() path already has lock acquired.
>>>> I think you need the lock only in 
>>>> CompileBroker::print_compile_queues().
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 9/17/14 1:54 AM, Nils Eliasson wrote:
>>>>>
>>>>> Hi Albert,
>>>>>
>>>>> On 2014-09-16 10:45, Albert wrote:
>>>>>> Hi Nils,
>>>>>>
>>>>>> please see comments inline.
>>>>>>
>>>>>> On 09/16/2014 10:16 AM, Nils Eliasson wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I would like review of this change that includes three fixes:
>>>>>>>
>>>>>>> 1) Let Dcmd Compiler.codelist only print alive-nmethods. We ran 
>>>>>>> into
>>>>>>> crashes when listing zombies and unloaded too. Alive nmethods
>>>>>>> includes not-entrants so it still gives a pretty good idea about
>>>>>>> whats in the code cache and what has been used recently.
>>>>>>>
>>>>>> What was the cause of these crashes? Is the reason that
>>>>>> nmethod->method() is null?
>>>>>
>>>>> In one case it crashed when nmethod->method()->constants() was 
>>>>> null when
>>>>> printing the name.
>>>>>
>>>>> I see that the segmented code cache push already changed to only
>>>>> iterating the alive nmethods so I removed that from the patch.
>>>>>
>>>>>> Would it make sense to print information about zombie methods that
>>>>>> don't lead
>>>>>> to a crash?
>>>>> Probably not. Not_entrant nmethods may stay around for a while, 
>>>>> but once
>>>>> zombies they are reclaimed pretty fast.
>>>>>
>>>>>>
>>>>>>
>>>>>>> 2) Take CompileQueue lock when printing queue. It is not enough 
>>>>>>> to be
>>>>>>> at a safepoint - the compiler threads may still mutate the list
>>>>>>> causing crashes.
>>>>>>>
>>>>>> Are we guaranteed to not run into a deadlock when we acquire the 
>>>>>> lock?
>>>>>
>>>>> Yes. It doesn't use any other lock.
>>>>>
>>>>> New webrev: http://cr.openjdk.java.net/~neliasso/8058461/webrev.05/
>>>>>
>>>>> Thanks Albert and Christian for your feedback.
>>>>>
>>>>> //Nils
>>>>>
>>>>>>
>>>>>> Best,
>>>>>> Albert
>>>>>>> 3) Relax the parsing of long hex-numbers in the test of codelist.
>>>>>>> High addresses (sparc) casues NumberFormatExceptions.
>>>>>>>
>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8058461
>>>>>>> webrev: http://cr.openjdk.java.net/~neliasso/8058461/webrev.04/
>>>>>>>
>>>>>>> Thanks!
>>>>>>> //Nils
>>>>>>
>>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20140918/521395ac/attachment-0001.html>