RFR (round 4), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector
Roman Kennke
rkennke at redhat.com
Tue Dec 4 08:00:55 UTC 2018
Hi Jini,
> Thank you for making the changes. The SA portion looks good to me.
Thank you!
> One
> nit though:
>
> In
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/HeapSummary.java,
> in printGCAlgorithm(), does displaying the nbr of Parallel GC threads
> not make sense for Shenandoah (like it is for G1, ZGC, etc)?
I suppose it does. I will add it.
Thanks,
Roman
> Thank you,
> Jini.
>
>
> On 12/4/2018 12:57 AM, Roman Kennke 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