RFR (round 4), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector
Vladimir Kozlov
vladimir.kozlov at oracle.com
Sun Dec 2 18:10:16 UTC 2018
On 12/2/18 8:55 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.
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