Hi Dan,
On 22/11/2017 10:12 AM, Daniel D. Daugherty wrote:
Greetings,
*** We are wrapping up code review on this project so it is time ***
*** for the various code reviewers to chime in one last time. ***
In this latest round, we had three different reviewers chime in so we're
doing delta webrevs for each of those resolutions:
David H's resolutions:
http://cr.openjdk.java.net/~dcubed/8167108-webrev/jdk10-10.09,10.10.0-delta/
mq comment for dholmes_CR:
- Fix indents, trailing spaces and various typos.
- Add descriptions for the '_cnt', '_max' and '_times" fields,
add impl notes to document the type choices.
src/hotspot/share/runtime/thread.cpp
3466 // range. Unsigned 64-bit would be more future proof, but 64-bit
atomic inc
3467 // isn't available everywhere (or is it?).
64-bit atomics should be available on all currently supported platforms
(the last time this was an issue was for PPC32 - and the atomic
templates should have filled in any previous holes in the allowed
types). But if it's always 64-bit then you'd have to use Atomic::load to
allow for 32-bit platforms.
These counts etc are all just for statistics so we aren't really
concerned about eventual overflows in long running VMs - right?
---
// # of parallel threads in
_smr_delete_lock->wait().
static uint _smr_delete_lock_wait_cnt;
// Max # of parallel threads in
_smr_delete_lock->wait().
why are the comments indented like that? I thought this is what Coleen
previously commented on. Please just start the comments in the first
column - no need for any indent. Thanks.
src/hotspot/share/runtime/globals.hpp change is white-space only so it
is only visible via the file's patch link.
Robin W's resolutions:
http://cr.openjdk.java.net/~dcubed/8167108-webrev/jdk10-10.10.0,10.10.1-delta/
mq comment for robinw_CR:
- Fix some inefficient code, update some comments, fix some indents,
and add some 'const' specifiers.
src/hotspot/share/runtime/vm_operations.hpp change is white-space
only so
it is only visible via the file's patch link.
All looks fine to me. Some good catches there from Robin!
Coleen's resolutions:
http://cr.openjdk.java.net/~dcubed/8167108-webrev/jdk10-10.10.1,10.10.2-delta/
mq comment for coleenp_CR:
- Change ThreadsList::_threads from 'mtGC' -> 'mtThread',
- add header comment to threadSMR.hpp file,
- cleanup JavaThreadIteratorWithHandle ctr,
- make ErrorHandling more efficient.
src/hotspot/share/runtime/threadSMR.hpp
+ // Thread Safe Memory Reclaimation (Thread-SMR) support.
Only one 'i' in Reclamation :)
Thanks,
David
------
Some folks prefer to see all of the delta webrevs together so here is
a delta webrev relative to jdk10-09-full:
http://cr.openjdk.java.net/~dcubed/8167108-webrev/jdk10-10.09,10.10.2-delta/
src/hotspot/share/runtime/globals.hpp and
src/hotspot/share/runtime/vm_operations.hpp changes are white-space only
so they are only visible via the file's patch link.
And, of course, some folks prefer to see everything all at once so here
is the latest full webrev:
http://cr.openjdk.java.net/~dcubed/8167108-webrev/jdk10-10.10-full/
Dan has submitted the bits for the usual Mach5 tier[1-5] testing. The
prelim Mach5 tier1 (JPRT equivalent) on these bits passed just fine...
The project is currently baselined on Jesper's 2017-11-17 PIT snapshot
so there will be some catching up to do before integration...
We welcome comments, suggestions and feedback.
Dan, Erik and Robbin
On 11/18/17 8:49 PM, Daniel D. Daugherty wrote:
Greetings,
Testing of the last round of changes revealed a hang in one of the new
TLH tests. Robbin has fixed the hang, updated the existing TLH test, and
added another TLH test for good measure.
Here is the updated full webrev:
http://cr.openjdk.java.net/~dcubed/8167108-webrev/jdk10-09-full/
Here is the updated delta webrev:
http://cr.openjdk.java.net/~dcubed/8167108-webrev/jdk10-09-delta/
Dan ran the bits thru the usual Mach5 tier[1-5] testing and there are
no unexpected failures.
We welcome comments, suggestions and feedback.
Dan, Erik and Robbin
On 11/15/17 3:32 PM, Daniel D. Daugherty wrote:
Greetings,
Robbin rebased the project last night/this morning to merge with Thread
Local Handshakes (TLH) and also picked up additional changesets up thru:
Changeset: fa736014cf28
Author: cjplummer
Date: 2017-11-14 18:08 -0800
URL:http://hg.openjdk.java.net/jdk/hs/rev/fa736014cf28
8191049: Add alternate version of pns() that is callable from within
hotspot source
Summary: added pns2() to debug.cpp
Reviewed-by: stuefe, gthornbr
This is the first time we've rebased the project to bits that are this
fresh (< 12 hours old at rebase time). We've done this because we think
we're done with this project and are in the final review-change-retest
cycle(s)... In other words, we're not planning on making any more major
changes for this project... :-)
*** Time for code reviewers to chime in on this thread! ***
Here is the updated full webrev:
http://cr.openjdk.java.net/~dcubed/8167108-webrev/jdk10-08-full/
Here's is the delta webrev from the 2017.11.10 rebase:
http://cr.openjdk.java.net/~dcubed/8167108-webrev/jdk10-08-delta/
Dan has submitted the bits for the usual Mach5 tier[1-5] testing
(and the new baseline also)...
We're expecting this round to be a little noisier than usual because
we did not rebase on a PIT snapshot so the new baseline has not been
through Jesper's usual care-and-feeding of integration_blockers, etc.
We welcome comments, suggestions and feedback.
Dan, Erik and Robbin
On 11/14/17 4:48 PM, Daniel D. Daugherty wrote:
Greetings,
I rebased the project to the 2017.11.10 jdk/hs PIT snapshot.
(Yes, we're playing chase-the-repo...)
Here is the updated full webrev:
http://cr.openjdk.java.net/~dcubed/8167108-webrev/jdk10-07-full/
Unlike the previous rebase, there were no changes required to the
open code to get the rebased bits to build so there is no delta
webrev.
These bits have been run through JPRT and I've submitted the usual
Mach5 tier[1-5] test run...
We welcome comments, suggestions and feedback.
Dan, Erik and Robbin
On 11/13/17 12:30 PM, Daniel D. Daugherty wrote:
Greetings,
I rebased the project to the 2017.10.26 jdk10/hs PIT snapshot.
Here are the updated webrevs:
Here's the mq comment for the change:
Rebase to 2017.10.25 PIT snapshot.
Here is the full webrev:
http://cr.openjdk.java.net/~dcubed/8167108-webrev/jdk10-06-full/
And here is the delta webrev:
http://cr.openjdk.java.net/~dcubed/8167108-webrev/jdk10-06-delta/
I ran the above bits throught Mach5 tier[1-5] testing over the holiday
weekend. Didn't see any issues in a quick look. Going to take a closer
look today.
We welcome comments, suggestions and feedback.
Dan, Erik and Robbin
On 11/8/17 1:05 PM, Daniel D. Daugherty wrote:
Greetings,
Resolving one of the code review comments (from both Stefan K and
Coleen)
on jdk10-04-full required quite a few changes so it is being done
as a
standalone patch and corresponding webrevs:
Here's the mq comment for the change:
stefank, coleenp CR - refactor most JavaThreadIterator usage to use
JavaThreadIteratorWithHandle.
Here is the full webrev:
http://cr.openjdk.java.net/~dcubed/8167108-webrev/jdk10-05-full/
And here is the delta webrev:
http://cr.openjdk.java.net/~dcubed/8167108-webrev/jdk10-05-delta/
We welcome comments, suggestions and feedback.
Dan, Erik and Robbin
On 10/9/17 3:41 PM, Daniel D. Daugherty wrote:
Greetings,
We have a (eXtra Large) fix for the following bug:
8167108 inconsistent handling of SR_lock can lead to crashes
https://bugs.openjdk.java.net/browse/JDK-8167108
This fix adds a Safe Memory Reclamation (SMR) mechanism based on
Hazard Pointers to manage JavaThread lifecycle.
Here's a PDF for the internal wiki that we've been using to describe
and track the work on this project:
http://cr.openjdk.java.net/~dcubed/8167108-webrev/SMR_and_JavaThread_Lifecycle-JDK10-04.pdf
Dan has noticed that the indenting is wrong in some of the code
quotes
in the PDF that are not present in the internal wiki. We don't
have a
solution for that problem yet.
Here's the webrev for current JDK10 version of this fix:
http://cr.openjdk.java.net/~dcubed/8167108-webrev/jdk10-04-full
This fix has been run through many rounds of JPRT and Mach5
tier[2-5]
testing, additional stress testing on Dan's Solaris X64 server, and
additional testing on Erik and Robbin's machines.
We welcome comments, suggestions and feedback.
Daniel Daugherty
Erik Osterlund
Robbin Ehn
