RFR: 8351091: Shenandoah: global marking context completeness is not accurately maintained [v3]

[William Kemper]

On Wed, 5 Mar 2025 21:54:08 GMT, Xiaolong Peng <xpeng at openjdk.org> wrote:

>> With the JEP 404: Generational Shenandoah implementation, there are generation specific marking completeness flags introduced, and the global marking context completeness flag is not updated at all after initialization, hence the global marking context completeness is not accurate anymore. This may cause expected behavior: [ShenandoahHeap::complete_marking_context()](https://github.com/openjdk/jdk/pull/23886/files#diff-d5ddf298c36b1c91bf33f9bff7bedcc063074edd68c298817f1fdf39d2ed970fL642) should throw assert error if the global marking context completeness flag is false, but now it always return the marking context even it marking is not complete, this may hide bugs where we expect the global/generational marking to be completed.
>> 
>> This change PR fix the bug in global marking context completeness flag, and update all the places using `ShenandoahHeap::complete_marking_context()` to use proper API.
>> 
>> ### Test
>> - [x] hotspot_gc_shenandoah
>> - [x] Tier 1
>> - [x]  Tier 2
>
> Xiaolong Peng has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Remove obsolete code comments
>  - Address review comments

If we always get the complete marking context directly through the generation, we can delete `ShenandoahHeap::complete_marking_context`.

src/hotspot/share/gc/shenandoah/heuristics/shenandoahHeuristics.cpp line 123:

> 121: #ifdef ASSERT
> 122:       bool reg_live = region->has_live();
> 123:       bool bm_live = heap->complete_marking_context(region)->is_marked(cast_to_oop(region->bottom()));

Could also use `heap->active_generation()->complete_marking_context()` here.

src/hotspot/share/gc/shenandoah/shenandoahGenerationalEvacuationTask.cpp line 172:

> 170: // contained herein.
> 171: void ShenandoahGenerationalEvacuationTask::promote_in_place(ShenandoahHeapRegion* region) {
> 172:   ShenandoahMarkingContext* const marking_context = _heap->complete_marking_context(region);

We shouldn't need to look up the generation for this region. It's being promoted so it must be young (in fact, this asserted a few lines down). Perhaps:

assert(_heap->young_generation()->is_mark_completed(), "Cannot promote without complete marking for young");
ShenandoahMarkingContext* const marking_context = _heap->marking_context();

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

Changes requested by wkemper (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23886#pullrequestreview-2665222915
PR Review Comment: https://git.openjdk.org/jdk/pull/23886#discussion_r1983818301
PR Review Comment: https://git.openjdk.org/jdk/pull/23886#discussion_r1983812569