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 (agarthwa...@twitter.com) and Carsten
Varming (varm...@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
