Hi Coleen, > http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/05/shenandoah-gc/src/hotspot/share/gc/shenandoah/vm_operations_shenandoah.cpp.html > > > Can you rename these to shenandoahVMOperations.hpp/cpp to match the > newly agreed upon naming convention for this? > > See 8214791: Consistently name gc files containing VM operations [Was: > Re: RFR (S): 8214791: Rename vm_operations_g1* files to g1VMOperations*]
Doing so: http://mail.openjdk.java.net/pipermail/shenandoah-dev/2018-December/008595.html I will integrate this in next round of webrevs. I expect the next will be the final round of webrevs. I got positive reviews for all shared-* parts, I'm waiting for shenandoah-* reviews from Shenandoah devs, plus Zhengyu+Thomas' TaskQueue stuff to arrive in upstream jdk. The CSR for Shenandoah flags has been approved, and the JEP should be moved to targeted JDK12 ~tomorrow. I intend/expect to push Shenandoah during the next couple of days, unless somebody speaks up until then :-) Thanks, Roman > > thanks, > Coleen > > On 12/4/18 3:37 PM, Roman Kennke wrote: >> Thanks, Leonid, for reviewing! >> >> Roman >> >> >>> Hi >>> >>> The shared tests changes looks good for me. Thank you for fixing and >>> testing different combinations. >>> >>> Leonid >>> >>>> On Dec 3, 2018, at 11:10 PM, Roman Kennke <rken...@redhat.com> wrote: >>>> >>>> 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 >>>>>>>> >