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

[Xiaolong Peng]

On Thu, 6 Mar 2025 22:11:26 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>>> If so, can we dispense with the forcing of global context's state into the contexts for the two generations?
>> 
>> I think we can do that if we deprecated the classical mode and only support generational Shenandoah, in classical mode, there is only global generation.
>
> I am not sure I follow. In the legacy (non-generational mode) we shouldn't care about the marking state of the old and young generations, just that of the GlobalGeneration. In the generational case, we explicitly track the marking state of the old and young generations explicitly. It sounds to me as if forcing the Old and Young marking states to the state of that of the GlobalGeneration must be exactly for the case where we are using Generational Shenandoah, and we are doing a Global collection? Indeed:
> 
> 
> void ShenandoahGlobalGeneration::set_mark_complete() {
>   ShenandoahGeneration::set_mark_complete();
>   if (ShenandoahHeap::heap()->mode()->is_generational()) {
>     ShenandoahGenerationalHeap* heap = ShenandoahGenerationalHeap::heap();
>     heap->young_generation()->set_mark_complete();
>     heap->old_generation()->set_mark_complete();
>   }
> }
> 
> 
> I am saying that each of Old, Young, and Global generations maintain their own mark completion state and use that to determine what they pass back in response to `complete_marking_context()`. This completely localizes all state rather than unnecessarily and confusingly coupling the states of these generations.
> 
> So, you remove the part in the `if` branch in the code above, which reduces to the default (or rather only) implementation in the base class, not requiring the over-ride of the Global generation's method for the generational case.
> 
> 
> void ShenandoahGeneration::set_mark_complete() {
>   _is_marking_complete.set();
> }
> 
> 
> It is possible that I am still missing the actual structure here that requires the override for GlobalGeneration for the generational case.

Sorry I misunderstood your original proposal, I thought you meant to suggest to remove the flag from  ShenandoahGlobalGeneration, instead the set_mark_complete/is_mark_complete will more like view/delegation layer like:

void ShenandoahGlobalGeneration::set_mark_complete() {
    ShenandoahGenerationalHeap* heap = ShenandoahGenerationalHeap::heap();
    heap->young_generation()->set_mark_complete();
    heap->old_generation()->set_mark_complete();
}

bool ShenandoahGlobalGeneration::is_mark_complete() {
    ShenandoahGenerationalHeap* heap = ShenandoahGenerationalHeap::heap();
    return heap->young_generation()->is_mark_complete() && heap->old_generation()->is_mark_complete();
}

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23886#discussion_r1984182699