RFR: 8357976: GenShen crash in swap_card_tables: Should be clean [v3]

[William Kemper]

On Mon, 16 Jun 2025 16:44:25 GMT, Xiaolong Peng <xpeng at openjdk.org> wrote:

>> @pengxiaolong - We don't want to clean the read table during init-mark because that's a safepoint. We can safely clean the read table concurrently because only the GC thread has access to it.
>> 
>> @ysramakrishna - The refactoring you suggest is not so simple, alas. We have to be very careful about the ordering of cleaning the `read` table, resetting the mark bitmap and swapping the card tables. Genshen does not clean the `read` table at the end of a concurrent cycle (we _could_ do this, but we'd still need to do the same in a degenerated cycle). The complexity comes from swapping the card tables in a concurrent cycle. We must not let a subsequent degenerated cycle swap them a second time. The degenerated cycle can only clean the `read` table if there was no active concurrent cycle (lest it throw away critical remembered set data). 
>> 
>> However, the degenerated cycle must also clear the mark bitmaps if the concurrent cycle degenerated during the root scan (the degenerated cycle will rescan the roots). Of course, we cannot clear the mark bitmaps if the concurrent cycle degenerated during the mark phase. So, for the degenerated cycle, clearing the mark bitmaps and cleaning the read table need to happen in two different cases and it doesn't work if they're combined into the same method (and I don't think adding a method parameter to sometimes clear the mark bitmap and sometimes the read table will make the code any easier to understand).
>
>> We don't want to clean the read table during init-mark because that's a safepoint. We can safely clean the read table concurrently because only the GC thread has access to it.
> 
> Thanks, we want to minimize the time at safepoint, I understand this part. My question was about swap_card_tables, I we don't have to do it at safepoint, we could move swap_card_tables out of the init-mark safepoint and move to close to mark_read_table_as_clean to make the code slight earlier to understand. @ysramakrishna also suggested this, I saw your attempt and rollback, thanks for the explanations.
> 
> It looks good to me.

@pengxiaolong - I think we _could_ swap card table pointers with a thread local handshake, but if we don't remove `init-mark` entirely, it would just mean stopping all the threads twice. I'm inclined to leave it on the safepoint for now.

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

PR Comment: https://git.openjdk.org/jdk/pull/25735#issuecomment-2978071741