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 >>> >> >
