RFR: JDK-8314777: GenShen: Alias young and old marking bits to legacy Shenandoah marking bit in gc state

[William Kemper]

On Tue, 22 Aug 2023 00:10:42 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

> This change sets the MARKING bit of _gc_state whenever either Young marking or Old marking is active.  With this change, barrier code more closely resembles the code in the original single-generation Shenandoah.  The performance impact is negligible.  The primary benefit is to simplify code reviews and clarify that the addition of generational mode to Shenandoah does not negatively impact performance of single-generation Shenandoah.

Changes requested by wkemper (Committer).

src/hotspot/share/gc/shenandoah/c2/shenandoahBarrierSetC2.cpp line 944:

> 942:     int flags = ShenandoahHeap::HAS_FORWARDED;
> 943:     if (ShenandoahIUBarrier) {
> 944:       assert(!ShenandoahHeap::heap()->mode()->is_generational(), "Generational mode does not support IU barrier");

Not sure we need an assert here, we could check this condition in `shenandoahGenerationalMode.cpp` or `shenandoahArguments.cpp` to minimize deltas from upstream.

src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 2460:

> 2458:   if (!in_progress && is_concurrent_old_mark_in_progress()) {
> 2459:     assert(mode()->is_generational(), "Only generational GC has old marking");
> 2460:     // If old-marking is in progress when we turn off YOUNG_MARKING, leave MARKING (and OLD_MARKING) on

Can we assert that gc state mask has `MARKING` set here?

src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 2468:

> 2466: }
> 2467: 
> 2468: void ShenandoahHeap::set_concurrent_old_mark_in_progress(bool in_progress) {

Should we assert that if `has_forwarded_objects` that the gc state mask has `UPDATEREFS` set?

src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 2470:

> 2468: void ShenandoahHeap::set_concurrent_old_mark_in_progress(bool in_progress) {
> 2469:   if (!in_progress && is_concurrent_young_mark_in_progress()) {
> 2470:     // If young-marking is in progress when we turn off OLD_MARKING, leave MARKING (and YOUNG_MARKING) on

Similarly, assert that `MARKING` is set here?

src/hotspot/share/gc/shenandoah/shenandoahHeap.inline.hpp line 661:

> 659: 
> 660: inline bool ShenandoahHeap::is_idle() const {
> 661:   return _gc_state.is_unset(MARKING | YOUNG_MARKING | OLD_MARKING | EVACUATION | UPDATEREFS);

We should just need to check `MARKING` now, right?

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

PR Review: https://git.openjdk.org/shenandoah/pull/309#pullrequestreview-1589843000
PR Review Comment: https://git.openjdk.org/shenandoah/pull/309#discussion_r1301892881
PR Review Comment: https://git.openjdk.org/shenandoah/pull/309#discussion_r1301885170
PR Review Comment: https://git.openjdk.org/shenandoah/pull/309#discussion_r1301883842
PR Review Comment: https://git.openjdk.org/shenandoah/pull/309#discussion_r1301887335
PR Review Comment: https://git.openjdk.org/shenandoah/pull/309#discussion_r1301888882