Hi Jini, Thanks for your suggestions. I've added this to Shenandoah's dev:
http://cr.openjdk.java.net/~rkennke/shenandoah-sa/webrev.01/ and it will show up in next round of webrevs. Thanks, Roman > A few comments on the SA changes: > > ==> Could you please add the following lines in > src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/HSDB.java from line > 1120 onwards to avoid the "[Unknown generation]" message with hsdb while > displaying the Stack Memory for a mutator thread ? > > else if (collHeap instanceof ShenandoahHeap) { > ShenandoahHeap heap = (ShenandoahHeap) collHeap; > anno = "ShenandoahHeap "; > bad = false; > } > > ==> > src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/HeapSummary.java > > The printGCAlgorithm() method would need to be updated to read in the > UseShenandoahGC flag to avoid the default "Mark Sweep Compact GC" being > displayed with jhsdb jmap -heap. > > ==> > src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/gc/shared/GCName.java > > Could you please add "Shenandoah" to the GCName enum list ? > > ==> > src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/gc/shared/GCCause.java > > Could you please update the GCCause enum values to include these: > > _shenandoah_stop_vm, > _shenandoah_allocation_failure_evac, > _shenandoah_concurrent_gc, > _shenandoah_traversal_gc, > _shenandoah_upgrade_to_full_gc, > > ==> share/classes/sun/jvm/hotspot/runtime/VMOps.java > > It would be good to add 'ShenandoahOperation' to the VMOps enum (though > it is probably not in sync now). > > Thank you, > Jini. > > On 12/1/2018 2:30 AM, Roman Kennke wrote: >> 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 >>>>> >>>> >>> >>
