RFR: 8255765: Shenandoah: Isolate concurrent, degenerated and full GC [v3]

[Aleksey Shipilev]

On Tue, 19 Jan 2021 19:34:02 GMT, Zhengyu Gu <zgu at openjdk.org> wrote:

>> The purpose of this patch is to isolate concurrent, degenerated and full gc implementation, so that, makes each GC implementation more straightforward and clean, and improves readability.
>> 
>> Current implementation emphasis code sharing, e.g. degenerated GC reuses concurrent GC's ops. It was not a problem in the beginning, when they actually behave similarly. Since concurrent GC moved root processing into concurrent phases, code started to diverge, we started to put bandages to make the shared ops work for both concurrent and degenerated GC, that made code hard to read and error prone.
>> 
>> The patch breaks up GCs into 3 (mainly just concurrent and degenerated GC, as full GC already standalone) easy to identify and understand classes (ShenandoahConcurrentGC, ShenandoahDegeneratedGC and ShenandoahMarkCompactGC), subclasses of ShenandoahGC class.
>> 
>> The three GCs still keep vmop/entry/op paradigm, but encapsulate GC control flow inside their own classes, as ShenandoahMarkCompact GC already does. So that, concurrent and degenerated GC no longer share ops and op implementations no longer need to consider other GC modes, which results in simplifying implementation and improving readability. Code sharing is achieved via helper methods provided by ShenandoahHeap.
>> 
>> Test:
>>  - [x] hotspot_gc_shenandoah
>>  - [x] nightly tests
>
> Zhengyu Gu has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 101 commits:
> 
>  - Merge branch 'master' into JDK-8255765-isolate-gcs
>  - Aleksey's comments
>  - Remove cached heap in ShenandoahGC
>  - Merge
>  - Merge branch 'fix_phase_timings' into JDK-8255765-isolate-gcs
>  - Merge
>  - Merge
>  - Merge
>  - Merge
>  - Removed trailing whitespaces
>  - ... and 91 more: https://git.openjdk.java.net/jdk/compare/3edf393d...4e54d38d

I have only minor nits left.

src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp line 412:

> 410: }
> 411: 
> 412: // Actual work for the phases

I think we can drop this comment.

src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp line 659:

> 657:     }
> 658: 
> 659:      _dedup_roots.prologue();

It looks to me this line in misindented: one stray space?

src/hotspot/share/gc/shenandoah/shenandoahDegeneratedGC.cpp line 115:

> 113:       // STW mark
> 114:       op_mark();
> 115:     case _degenerated_mark:

New line before this `case`?

src/hotspot/share/gc/shenandoah/shenandoahGC.hpp line 66:

> 64: };
> 65: 
> 66: #endif

Should be `#endif // SHARE_GC_SHENANDOAH_SHENANDOAHGC_HPP`?

-------------

Marked as reviewed by shade (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1964