RFR (round 1), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector

[Roman Kennke]

Hi Vladimir,

thanks for reviewing!

> .ad files. I am thinking may be we should put new code in separate
> <arch>_shenandoah.ad files and merge them only if Shenandoah is included
> in built [1]. I don't like #ifdefs - you still generate this mach nodes
> even without Shenandoah.

We're looking into improving the situation.

> debug_final_field_at() and debug_stable_field_at() are not used - could
> be excluded from changes.

Those are used from shenandoahBarrierSetC2.cpp and related code in in
the shenandoah-gc or shenandoah-complete webrevs.

> loopnode.cpp Sometimes you add #if INCLUDE_SHENANDOAHGC and sometimes
> don't. Be consistent. I don't know why you need #ifdef if such nodes
> should be generated without Shenandoah

We'll look into improving that.

> compile.cpp - no need to include shenandoahBarrierSetC2.hpp

Right. Will fix it.

> type.hpp - cast_to_nonconst() are not used.

As above, this is used from shenandoahBarrierSetC2.cpp

> phasetype.hpp, subnode.cpp - don't add files with only cleanup changes.

right. Those are leftovers and I'll remove them.

> I wish loopnode.cpp and loopopts.cpp changes were more clean.

We'll see what we can do. Some of those could possibly be turned into
some sort of GC interface, but 1. it would be very Shenandoah-specific
and 2. require significant duplication of code.

> I like the idea where you expose only one ShenandoahBarrier node to C2
> and hide all subnodes behind it. I really don't want to see a lot of
> Shenandoah*Node new classes in C2 shared code.

I understand that. We have ShenandoahBarrierNode superclass and a bunch
of ShenandoahReadBarrier, WriteBarrier and some other nodes 'behind'
this. The crux is probably all the various ShenandoahCompareAndSwap etc
variants, which need to be subclasses of their
non-Shenandoah-counterparts, and I don't see how to change that.

Thanks for reviewing and helping!
Roman

> 
> [1]
> http://hg.openjdk.java.net/jdk/jdk/file/157c1130b46e/make/hotspot/gensrc/GensrcAdlc.gmk#l122
> 
> 
> On 11/28/18 2:29 PM, Roman Kennke wrote:
>> 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
>>>
>>