RFR (XXL) [7u60]: nmethod backports (12 backports) (original) (raw)
Thomas Schatzl thomas.schatzl at oracle.com
Sat Jan 18 23:09:05 UTC 2014
- Previous message (by thread): JDK7 Minor GC pause as compared to JDK6
- Next message (by thread): RFR (XXL) [7u60]: nmethod backports (12 backports)
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Hi all,
please review the backport of the nmethod optimization changes for G1 (7145569) and all related issues.
This request in total contains 12 changes, five which enable easy backport of the above mentioned main change, and six that fix issues with the original change.
These are
Add new flag for verifying the heap during startup (8029486) Add a flag to turn off the output of the verbose verification code (8029487) G1: Add remembered set size information to output of G1PrintRegionLivenessInfo (8028187) G1: G1SummarizeRSetStats output on Linux needs improvement (8028186) G1: Verification after a full GC is incorrectly placed. (8029596) G1: optimize nmethods scanning (8025423) G1: G1CollectedHeap::mark_strong_code_roots() needs to handle ParallelGCThreads=0 (8028193) G1: improve remembered set summary information by providing per region type information (8028188) G1: assert "assert(thread < _num_vtimes) failed: just checking" fails when G1ConcRefinementThreads > ParallelGCThreads (8028189) -XX:+G1SummarizeRSetStats can result in wrong exit code and crash (8028190) assert(!hr->isHumongous()) failed: code root in humongous region? (8028191) Test gc/g1/TestPrintRegionRememberedSetInfo.java fails with "test result: Error. No action after @build" (8029488)
To track these backport CRs, the original CRs and their webrevs, I created an overview page for all changes at http://cr.openjdk.java.net/~tschatzl/nmethod-backport/ , listing the jdk 7u60 bug IDs and links to webrevs, and the corresponding original jdk 8 bug IDs and webrevs.
Except for the main change (JDK-8025423, JDK-7145569) there were no particular changes necessary (possibly adding a missing method from another change verbatim), so I think reviews should ooncentrate on that. Of course, feel free to comment on the other backports.
Some notes about the backport:
jdk7 has the old perm gen, so some changes simply did not apply at all. E.g. there is no YoungRefCounterClosure to move, no VerifyKlassClosure etc.
there is one important change in how G1CollectedHeap::g1_process_strong_roots() and SharedHeap::process_strong_roots() are called.
hs24(jdk7) and hs25(jdk8) handle code cache roots differently. In SharedHeap::process_strong_roots() there is the option that the caller sets SO_CodeCache to scan code cache roots. The problem is that even if SO_CodeCache is not set, jdk7 iterates over some of the code roots that have references into the heap anyway. Additionally this is also dependent on whether we are collecting the perm gen or not.
This is different to hs25 where SharedHeap::process_strong_roots() only ever iterates code roots when SO_CodeCache is set.
Now, with the nmethod optimization, we do not want SharedHeap::process_strong_roots() to do any code root iteration like in JDK8.
This unfortunately changed as part of the (huge) initial perm gen removal patch.
Instead of trying to reorganizing this code, and looking through all invocations to SharedHeap::process_strong_roots() to set the correct flags (plus verification), I introduced a new boolean to SharedHeap::process_strong_roots() called "manages_code_roots" that indicates whether the caller handles code roots by itself in the case when we are not explicitly passing SO_CodeCache.
This flag is only set to true (with a default value of false) in case of G1 young collection, so minimizing code changes.
As far as I can see, the alternative is to implement something like in JDK8 here: add a is_scavengable flag and look through all invocations to process_strong_roots() to pass the SO_CodeCache flag.
As mentioned, I tried to minimize changes here, I am open to suggestions here to improve that part though.
in (Un-)RegisterNMethodOopClosure::do_oop_work() the code needs to ignore references into the permanent generation. In JDK8 these closures do not get references that are embedded into the code into the "perm gen" any more. They are simply not oops.
added GrowableArray::find_from_end() and GrowableArray::max_length(), also from that initial perm gen removal change.
for the backport in JDK-8028191 I had to add a "deoptimizeAll" Whitebox method that calls a compiler method to deoptimize all code. This was part of a larger compiler change that has not been backported.
Improvement: Reduces young gc pause time in some FMW applications by a significant amount, particularly for code that has a significant amount of compiled code, e.g. from ~160ms to ~30ms for young-only GCs in CRM Fuse.
Testing: jtreg, jprt, FMW applications, dacapo, specjbb05, ad-hoc (adhoc runs with default collector and G1, fastdebug + product) testing
Comparing to a vanilla 7u60 adhoc testing run, there were three occurrences of https://bugs.openjdk.java.net/browse/JDK-8030803 that has been fixed in jdk8 but apparently is not scheduled for 7u60. It is completely unrelated to the changes from what I can see, and a known issue in 7u60.
Thanks a lot, Thomas
- Previous message (by thread): JDK7 Minor GC pause as compared to JDK6
- Next message (by thread): RFR (XXL) [7u60]: nmethod backports (12 backports)
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]