Zhengyu's and Thomas' TaskQueue stuff landed in jdk/jdk and we integrated it into shenandoah/jdk and in the master patch and webrevs. I'm updating the webrevs in-place in:
http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/06/ This is going to be the final webrev that I intend to push on Monday, unless somebody stops me or we face serious merge conflicts by then. I've just pushed this to jdk/submit for testing. I will also do more testing locally over the weekend. Thanks everybody for reviewing and helping and being patient! Cheers, Roman > Here comes round 6, possibly/hopefully the last round of webrevs for > upstreaming Shenandoah: > > http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/06/ > > It incorporates: > - renames vm_operations_shenandoah* to shenandoahVMOperations*, as > suggested by Coleen > - reshuffles gcCause in shared-gc SA as suggested by Per > - print number of threads in SA, as suggested by Jini > - fixed a problem in webrev generation that did not track move of > CriticalNativeStress.java and CriticalNativeArgs.java > > The CSR has been approved, the JEP moved to target jdk12, and I got > positive reviews for all parts. I intend to push this early next week, > unless somebody stops me. If Zhengyu's and Thomas' TaskQueue change goes > in by then, I'll incorporate it. > > Thanks everybody for reviewing and reviewing and reviewing again ;-) > > Cheers, > Roman > >> Round 5 of Shenandoah review includes: >> - A fix for the @requires tag in TestFullGCCountTest.java. It should be >> correct now. We believe the CMS @requires was also not quite right and >> fixed it the same. >> >> It reads now: Don't run this test if: >> - Actual GC set by harness is CMS *and* ExplicitGCInvokesConcurrent is >> true, as set by harness >> - Actual GC set by harness is Shenandoah *and* >> ExplicitGCInvokesConcurrent is not set false by harness (it's true by >> default in Shenandoah, so this needs to be double-inverteed). >> >> The @requires for CMS was wrong before (we think), because it would also >> filter defaultGC + ExplicitGCInvokesConcurrent. >> >> - Sorting of macros was fixed, as was pointed out by Per >> - Some stuff was added to SA, as suggested by Jini >> - Rebased on most current jdk/jdk code >> >> Webrevs: >> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/05/ >> >> I also need reviews from GC reviewers for the CSR: >> https://bugs.openjdk.java.net/browse/JDK-8214349 >> >> I already got reviews for: >> [x] shared-runtime (coleenp) >> [x] shared-compiler (kvn) >> >> I got reviews for shared-build, but an earlier version, so maybe makes >> sense to look over this again. Erik J, Magnus? >> >> I still need approvals for: >> [ ] shared-build (kvn, erikj, ihse, pliden) >> [ ] shared-gc (pliden, kbarrett) >> [ ] shared-serviceability (jgeorge, pliden) >> [ ] shared-tests (lmesnik, pliden) >> [ ] shenandoah-gc >> [ ] shenandoah-tests >> >> >> Thanks for your patience and ongoing support! >> >> Cheers, >> Roman >> >> >>> Hi all, >>> >>> here comes round 4 of Shenandoah upstreaming review: >>> >>> This includes fixes for the issues that Per brought up: >>> - Verify and gracefully reject dangerous flags combinations that >>> disables required barriers >>> - Revisited @requires filters in tests >>> - Trim unused code from Shenandoah's SA impl >>> - Move ShenandoahGCTracer to gc/shenandoah >>> - Fix ordering of GC names in various files >>> - Rename UINT64_FORMAT_HEX_W to UINT64_FORMAT_X_W >>> >>> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/04/ >>> >>> Thanks everybody for taking time to review this! >>> Roman >>> >>>> Hello all, >>>> >>>> Thanks so far for all the reviews and support! >>>> >>>> I forgot to update the 'round' yesterday. We are in round 3 now :-) >>>> Also, I fixed the numbering of my webrevs to match the review-round. ;-) >>>> >>>> Things we've changed today: >>>> - We moved shenandoah-specific code out of .ad files into our own .ad >>>> files under gc/shenandoah (in shenandoah-gc), how cool is that? This >>>> requires an addition in build machinery though, see >>>> make/hotspot/gensrc/GensrcAdlc.gmk (in shared-build). >>>> - Improved zero-disabling and build-code-simplification as suggested by >>>> Magnus and Per >>>> - Cleaned up some leftovers in C2 >>>> - Improved C2 loop opts code by introducing another APIs in >>>> BarrierSetC2. See the new APIs in shared-gc under BarrierSetC2.hpp. >>>> - I don't see where it makes sense to put INCLUDE_SHENANDOAHGC guards now. >>>> - We would all very much prefer to keep ShenandoahXYZNode names, as >>>> noted earlier. This stuff is Shenandoah-specific, so let's just call it >>>> that. >>>> - Rehashed Shenandoah tests (formatting, naming, directory layout, etc) >>>> - Rebased on jdk-12+22 >>>> >>>> - Question: let us know if you need separate RFE for the new BSC2 APIs? >>>> >>>> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/03/ >>>> >>>> Thanks, >>>> Roman >>>> >>>>> Alright, we fixed: >>>>> - The minor issues that Kim reported in shared-gc >>>>> - A lot of fixes in shared-tests according to Leonid's review >>>>> - Disabled SA heapdumping similar to ZGC as Per suggested >>>>> >>>>> Some notes: >>>>> Leonid: test/hotspot/jtreg/gc/TestFullGCCount.java was actually >>>>> correct. The @requires there means to exclude runs with both CMS and >>>>> ExplicitGCInvokesConcurrent at the same time, because that would be >>>>> (expectedly) failing. It can run CMS, default GC and any other GC just >>>>> fine. Adding the same clause for Shenandoah means the same, and filters >>>>> the combination (+UseShenandoahGC)+(+ExplicitGCInvokesConcurrent). I >>>>> made the condition a bit clearer by avoiding triple-negation. >>>>> >>>>> See: >>>>> http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-November/008457.html >>>>> >>>>> Per: Disabling the SA part for heapdumping makes 2 tests fail: >>>>> - test/hotspot/jtreg/serviceability/sa/ClhsdbJhisto.java >>>>> - test/hotspot/jtreg/serviceability/sa/TestHeapDumpForLargeArray.java >>>>> >>>>> we filter them for Shenandoah now. I'm wondering: how do you get past >>>>> those with ZGC? >>>>> >>>>> See: >>>>> http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-November/008466.html >>>>> >>>>> (Note to Leonid and tests reviewers: I'll add those related filters in >>>>> next round). >>>>> >>>>> Vladimir: Roland integrated a bunch of changes to make loop* code look >>>>> better. I can tell that we're not done with C2 yet. Can you look over >>>>> the code and see what is ok, and especially what is not ok, so that we >>>>> can focus our efforts on the relevant parts? >>>>> >>>>> Updated set of webrevs: >>>>> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/01/ >>>>> >>>>> Thanks, >>>>> Roman >>>>> >>>>> >>>>>> Hi, >>>>>> >>>>>> This is the first round of changes for including Shenandoah GC into >>>>>> mainline. >>>>>> I divided the review into parts that roughly correspond to the mailing >>>>>> lists >>>>>> that would normally review it, and I divided it into 'shared' code >>>>>> changes and >>>>>> 'shenandoah' code changes (actually, mostly additions). The intend is to >>>>>> eventually >>>>>> push them as single 'combined' changeset, once reviewed. >>>>>> >>>>>> JEP: >>>>>> https://openjdk.java.net/jeps/189 >>>>>> Bug entry: >>>>>> >>>>>> https://bugs.openjdk.java.net/browse/JDK-8214259 >>>>>> >>>>>> Webrevs: >>>>>> http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/ >>>>>> >>>>>> For those who want to see the full change, have a look at the >>>>>> shenandoah-complete >>>>>> <http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shenandoah-complete/> >>>>>> directory, >>>>>> it contains the full combined webrev. Alternatively, there is the file >>>>>> shenandoah-master.patch >>>>>> <http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shenandoah-master.patch>, >>>>>> which is what I intend to commit (and which should be equivalent to the >>>>>> 'shenandoah-complete' webrev). >>>>>> >>>>>> Sections to review (at this point) are the following: >>>>>> *) shenandoah-gc >>>>>> <http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shenandoah-gc/> >>>>>> - Actual Shenandoah implementation, almost completely residing in >>>>>> gc/shenandoah >>>>>> >>>>>> *) shared-gc >>>>>> <http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-gc/> >>>>>> - This is mostly boilerplate that is common to any GC >>>>>> - referenceProcessor.cpp has a little change to make one assert not >>>>>> fail (next to CMS and G1) >>>>>> - taskqueue.hpp has some small adjustments to enable subclassing >>>>>> >>>>>> *) shared-serviceability >>>>>> <http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-serviceability/> >>>>>> - The usual code to support another GC >>>>>> >>>>>> *) shared-runtime >>>>>> <http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-runtime/> >>>>>> - A number of friends declarations to allow Shenandoah iterators to >>>>>> hook up with, >>>>>> e.g. ClassLoaderData, CodeCache, etc >>>>>> - Warning and disabling JFR LeakProfiler >>>>>> - fieldDescriptor.hpp added is_stable() accessor, for use in >>>>>> Shenandoah C2 optimizations >>>>>> - Locks initialization in mutexLocker.cpp as usual >>>>>> - VM operations defines for Shenandoah's VM ops >>>>>> - globalDefinitions.hpp added UINT64_FORMAT_HEX_W for use in >>>>>> Shenandoah's logging >>>>>> - The usual macros in macro.hpp >>>>>> >>>>>> *) shared-build >>>>>> <http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-build/> >>>>>> - Add shenandoah feature, enabled by default, as agreed with >>>>>> Vladimir K. beforehand >>>>>> - Some flags for shenandoah-enabled compilation to get >>>>>> SUPPORT_BARRIER_ON_PRIMITIVES >>>>>> and SUPPORT_NOT_TO_SPACE_INVARIANT which is required for >>>>>> Shenandoah's barriers >>>>>> - --param inline-unit-growth=1000 settings for 2 shenandoah source >>>>>> files, which is >>>>>> useful to get the whole marking loop inlined (observed significant >>>>>> regression if we >>>>>> don't) >>>>>> >>>>>> *) shared-tests >>>>>> <http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shared-tests/> >>>>>> - Test infrastructure to support Shenandoah >>>>>> - Shenandoah test groups >>>>>> - Exclude Shenandoah in various tests that can be run with selected >>>>>> GC >>>>>> - Enable/add configure for Shenandoah for tests that make sense to >>>>>> run with it >>>>>> >>>>>> *) shenandoah-tests >>>>>> <http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/00/shenandoah-tests/> >>>>>> - Shenandoah specific tests, most reside in gc/shenandoah >>>>>> subdirectory >>>>>> - A couple of tests configurations have been added, e.g. >>>>>> TestGCBasherWithShenandoah.java >>>>>> >>>>>> I intentionally left out shared-compiler for now, because we have some >>>>>> work left to do >>>>>> there, but if you click around you'll find the patch anyway, in case you >>>>>> want to take >>>>>> a peek at it. >>>>>> >>>>>> We have regular builds on: >>>>>> - {Linux} x {x86_64, x86_32, armhf, aarch64, ppc64el, s390x} >>>>>> - {Windows} x {x86_64}, >>>>>> - {MacOS X} x {x86_64} >>>>>> >>>>>> This also routinely passes: >>>>>> - the new Shenandoah tests >>>>>> - jcstress with/without aggressive Shenandoah verification >>>>>> - specjvm2008 with/without aggressive Shenandoah verification >>>>>> >>>>>> >>>>>> I'd like to thank my collegues at Red Hat: Christine Flood, she deserves >>>>>> the credit for being the original inventor of Shenandoah, Aleksey >>>>>> Shiplëv, Roland Westrelin & Zhengyu Gu for their countless >>>>>> contributions, everybody else in Red Hat's OpenJDK team for testing, >>>>>> advice and support, my collegues in Oracle's GC, runtime and compiler >>>>>> teams for tirelessly helping with and reviewing all the GC interface and >>>>>> related changes, and of course the many early adopters for reporting >>>>>> bugs and success stories and feature requests: we wouldn't be here >>>>>> without any of you! >>>>>> >>>>>> Best regards, >>>>>> Roman >>>>>> >>>>> >>>> >>> >> >
