RFR (round 4), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector
Roman Kennke
rkennke at redhat.com
Sun Dec 2 09:57:35 UTC 2018
Hi Vladimir,
> Much better.
Thanks!
> Why you need to include shenandoahBarrierSetC2.hpp in /opto/classes.cpp
> ? Why not include only shenandoahSupport.hpp when new nodes are defined?
For no particular reason, I guess. I'll fix it in next round.
> I think it is fine to not use #ifdef in loopopts.cpp when you check
> is_ShenandoahBarrier(). And you don't do that in other files.
>
> Code in opto/macro.cpp is ugly but it is only the place so we can live
> with it I think.
Ok, thanks! We'll see if we can improve macro.cpp.
Thanks,
Roman
> Thanks,
> Vladimir
>
> On 11/30/18 1:00 PM, 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
>>>>>
>>>>
>>>
>>
More information about the build-dev
mailing list