RFR(XS) for PeriodicTask_lock cleanup (8072439)

[Coleen Phillimore]

Hi, I think you have better reviewer for this now but I did have a 
couple of questions.

in 
http://cr.openjdk.java.net/~dcubed/8072439-webrev/2-for_jdk9_hs_rt/src/share/vm/runtime/task.cpp.udiff.html

This comment is really confusing because I think you do in fact honor 
the safepoint protocol sometimes for the WatcherThread, true?

   65     // The WatcherThread is not a JavaThread so we do not honor the
   66     // safepoint protocol for the PeriodicTask_lock.
   67     MutexLockerEx ml(PeriodicTask_lock, Mutex::_no_safepoint_check_flag);


Because WatcherThread::stop() does safepoint check.   It's unclear 
whether enroll and disenroll can be called by the WatcherThread also.  
If so, should they have no_safepoint_check.

The whole safepoint checking inconsistency still makes me nervous, but 
the comment seems misleading.

I honestly don't know enough about the rest to review it.

thanks,
Coleen

On 2/25/15, 12:00 PM, Daniel D. Daugherty wrote:
> This should be the last webrev:
>
> http://cr.openjdk.java.net/~dcubed/8072439-webrev/2-for_jdk9_hs_rt/
>
> Coleen, since you were one of my reviewers on JDK-8047720, I'd like
> to hear from you in this hopefully final round...
>
> Dan
>
>
>
> On 2/18/15 10:00 AM, Daniel D. Daugherty wrote:
>> Greetings,
>>
>> Here is an updated webrev after addressing David H's comments:
>>
>> http://cr.openjdk.java.net/~dcubed/8072439-webrev/1-for_jdk9_hs_rt/
>>
>> Also, here is the bug's URL:
>>
>> JDK-8072439 fix for 8047720 may need more work
>> https://bugs.openjdk.java.net/browse/JDK-8072439
>>
>> Update for testing: I'm taking the new Remote Build and Test (RBT)
>> system for a ride during its beta period so I won't be doing direct
>> Aurora Adhoc jobs...
>>
>> Dan
>>
>>
>> On 2/17/15 2:44 PM, Daniel D. Daugherty wrote:
>>> Greetings,
>>>
>>> My fix for the following bug:
>>>
>>>     JDK-8047720 Xprof hangs on Solaris
>>>
>>> that was pushed to JDK9 last June needs to be cleaned up.
>>>
>>> Thanks to Alex Garthwaite (agarthwaite at twitter.com) and Carsten
>>> Varming (varming at gmail.com) for reporting the mess that I made
>>> in WatcherThread::stop() and for suggesting fixes.
>>>
>>> This code review is for a general cleanup pass on PeriodicTask_lock
>>> and some of the surrounding code. This is a targeted review in that
>>> I would like to hear from three groups of people:
>>>
>>> 1) The author and reviewers for:
>>>
>>>    JDK-7127792 Add the ability to change an existing PeriodicTask's
>>>                execution interval
>>>
>>>    Rickard, David H, and Markus G.
>>>
>>> 2) The reviewers for:
>>>
>>>    JDK-8047720 Xprof hangs on Solaris
>>>
>>>    Markus G and Coleen
>>>
>>> 3) Alex and Carsten
>>>
>>>
>>> Here's the webrev URL:
>>>
>>> http://cr.openjdk.java.net/~dcubed/8072439-webrev/0-for_jdk9_hs_rt/
>>>
>>> I've attached the original RFR for JDK-8047720 that explains
>>> the original deadlock that was being fixed. Similar testing
>>> will be done with this fix.
>>>
>>> Dan
>>
>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20150225/46782960/attachment.html>