RFR (round 4), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector
Roman Kennke
rkennke at redhat.com
Sun Dec 2 18:15:15 UTC 2018
>> 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