RFR (round 4), JDK-8214259: Implementation: JEP 189: Shenandoah: A Low-Pause Garbage Collector
Jini George
jini.george at oracle.com
Tue Dec 4 07:23:55 UTC 2018
Hi Roman,
Thank you for making the changes. The SA portion looks good to me. One
nit though:
In
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/HeapSummary.java,
in printGCAlgorithm(), does displaying the nbr of Parallel GC threads
not make sense for Shenandoah (like it is for G1, ZGC, etc)?
Thank you,
Jini.
On 12/4/2018 12:57 AM, Roman Kennke wrote:
> 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