RFR: 8369068: GenShen: Generations still aren't reconciled assertion failure [v2]

[Y. Srinivas Ramakrishna]

On Wed, 22 Oct 2025 00:29:29 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> William Kemper has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains nine commits:
>> 
>>  - Merge remote-tracking branch 'jdk/master' into reduce-gc_generation-usage
>>  - Merge remote-tracking branch 'jdk/master' into reduce-gc_generation-usage
>>  - Remove _gc_generation from ShenandoahHeap
>>  - Little cleanup, remove one active generation usage
>>  - Merge remote-tracking branch 'jdk/master' into reduce-gc_generation-usage
>>  - Finish removing usages of gc_generation, start on reducing usages of active_generation
>>  - Fix build
>>  - Use existing _generation field instead of Heap::_gc_generation where possible
>>  - Only shenandoah vm operations participate in active/gc generation scheme
>
> src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp line 358:
> 
>> 356:     assert(_from_region != nullptr, "must set before work");
>> 357:     assert(_heap->marking_context()->is_marked(p), "must be marked");
>> 358:     assert(!_heap->marking_context()->allocated_after_mark_start(p), "must be truly marked");
> 
> Why not `_generation->complete_marking_context()...` with the help of a `_generation` field kept in the object? It would make the idiom uniform and consistent across all these closures.

Or like you did below at line 784, perhaps `_heap->global_generation()->complete_marking_context()...` stashed in local `_ctx` field ?

> src/hotspot/share/gc/shenandoah/shenandoahGenerationalFullGC.cpp line 56:
> 
>> 54:   auto heap = ShenandoahGenerationalHeap::heap();
>> 55:   // Since we may arrive here from degenerated GC failure of either young or old, establish generation as GLOBAL.
>> 56:   heap->set_active_generation(heap->global_generation());
> 
> While this is good practice for consistency, is `_active_generation`  ever used by the worker threads when doing a full gc? Not asking to remove this line, just wondering if it's ever used now, following your changes in this PR.

PS: My reasoning is that we are doing this at a safepoint, presumably, and the full gc will be done by the time we get out of the safepoint. Thus, mutators will never see this value. Further, worker threads seem now to use a `generation` member field passed in with the relevant work closure/method.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27703#discussion_r2450047381
PR Review Comment: https://git.openjdk.org/jdk/pull/27703#discussion_r2450061214