RFR (round 5), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector
Roman Kennke
rkennke at redhat.com
Thu Dec 6 14:36:24 UTC 2018
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 <rkennke at 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
>>>>>>>>
>
More information about the build-dev
mailing list