RFR: 8357976: GenShen crash in swap_card_tables: Should be clean

[Y. Srinivas Ramakrishna]

On Tue, 10 Jun 2025 20:28:21 GMT, Xiaolong Peng <xpeng at openjdk.org> wrote:

>> Degenerated cycles must also clean the `read` card table before swapping it with the `write` card table.
>
> src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp line 646:
> 
>> 644:   if (heap->mode()->is_generational()) {
>> 645:     heap->old_generation()->card_scan()->mark_read_table_as_clean();
>> 646:   }
> 
> I guess swap_card_tables has to be done at safe-point, otherwise we can move this to the place right before swap_card_tables is called in `op_init_mark`

To the point that Xiaolong is making in his other comment, I think we can move this cleaning into the `prepare_gc()` for the young generation which would be called above.

Similarly, see my comment at the other place in degen collection, we'd have the prepare_gc() in the op_reset() clear the rad table along that path.

I think that is the right place to do this resetting as we "prepare" the generation for a future collection at the conclusion of this one.

> src/hotspot/share/gc/shenandoah/shenandoahDegeneratedGC.cpp line 142:
> 
>> 140:         // Clean the read table before swapping it. The end goal here is to have a clean
>> 141:         // write table, and to have the read table updated with the previous write table.
>> 142:         heap->old_generation()->card_scan()->mark_read_table_as_clean();
> 
> Should we clean only when _generation is young, given we swap card tables when _generation is young?

If we started out with a clean read table, each concurrent gc cleaned the read table after it completed, then if we degenerate outside a cycle, we should find the read table clean already. 

Unless I am missing something, we should be able to assert here that the read table is clean (which the swap already does). Do we need the reset?

It would seem that in the event that we degenerated within a concurrent cycle then we may not have done the reset and that's where we'd need to do an explicit reset further below where we call `op_reset()` which then calls `prepare_gc()` on the generation that was subject to this collection cycle, addressing the point that Xiaolong makes. See my comment in the other file as well.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25735#discussion_r2138918518
PR Review Comment: https://git.openjdk.org/jdk/pull/25735#discussion_r2138919561