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

Vladimir Kozlov vladimir.kozlov at oracle.com
Sun Dec 2 20:37:31 UTC 2018


Yes, compiler changes reviewed.

Vladimir

On 12/2/18 10:15 AM, Roman Kennke wrote:
>>> Hello Vladimir,
>>>
>>>> Why you need to include shenandoahBarrierSetC2.hpp in /opto/classes.cpp
>>>> ? Why not include only shenandoahSupport.hpp when new nodes are defined?
>>>
>>> I discussed this with my team. shenandoahBarrierSetC2.hpp is supposed to
>>> be the entry point for external C2 code. No C2 code is supposed to
>>> include shenandoahSupport.hpp, this is just an implementation detail of
>>> shenandoahBarrierSetC2.hpp. This seems symmetrical to how ZGC includes
>>> zBarrierSetC2.hpp from external C2 code. Is that ok to leave as it is?
>>
>> Yes, it is fine.
> 
> Ok great! Thanks!
> 
> Can I consider the shared-compiler part reviewed by you then?
> 
> Roman
> 
> 
>>
>> Vladimir
>>
>>>
>>> Cheers,
>>> Roman
>>>
>>>
>>>>
>>>> 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.
>>>>
>>>> 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