RFR (round 5), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector
Roman Kennke
rkennke at redhat.com
Tue Dec 4 07:10:28 UTC 2018
Round 5 of Shenandoah review includes:
- A fix for the @requires tag in TestFullGCCountTest.java. It should be
correct now. We believe the CMS @requires was also not quite right and
fixed it the same.
It reads now: Don't run this test if:
- Actual GC set by harness is CMS *and* ExplicitGCInvokesConcurrent is
true, as set by harness
- Actual GC set by harness is Shenandoah *and*
ExplicitGCInvokesConcurrent is not set false by harness (it's true by
default in Shenandoah, so this needs to be double-inverteed).
The @requires for CMS was wrong before (we think), because it would also
filter defaultGC + ExplicitGCInvokesConcurrent.
- Sorting of macros was fixed, as was pointed out by Per
- Some stuff was added to SA, as suggested by Jini
- Rebased on most current jdk/jdk code
Webrevs:
http://cr.openjdk.java.net/~rkennke/shenandoah-upstream/05/
I also need reviews from GC reviewers for the CSR:
https://bugs.openjdk.java.net/browse/JDK-8214349
I already got reviews for:
[x] shared-runtime (coleenp)
[x] shared-compiler (kvn)
I got reviews for shared-build, but an earlier version, so maybe makes
sense to look over this again. Erik J, Magnus?
I still need approvals for:
[ ] shared-build (kvn, erikj, ihse, pliden)
[ ] shared-gc (pliden, kbarrett)
[ ] shared-serviceability (jgeorge, pliden)
[ ] shared-tests (lmesnik, pliden)
[ ] shenandoah-gc
[ ] shenandoah-tests
Thanks for your patience and ongoing support!
Cheers,
Roman
> 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