RFR: 8337511: Implement JEP 404: Generational Shenandoah (Experimental) [v6]

[William Kemper]

On Wed, 13 Nov 2024 12:23:26 GMT, Aleksey Shipilev <shade 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 503 commits:
>> 
>>  - Merge openjdk/jdk tip into great-genshen-pr-redux
>>  - Merge remote-tracking branch 'jdk/master' into merge-latest
>>  - Merge remote-tracking branch 'jdk/master' into merge-latest
>>  - Merge
>>  - 8342861: GenShen: Old generation in unexpected state when abandoning mixed gc candidates
>>    
>>    Reviewed-by: kdnilsen
>>  - 8342734: GenShen: Test failure gc/shenandoah/TestReferenceRefersToShenandoah.java#generational
>>    
>>    Reviewed-by: ysr
>>  - 8342919: GenShen: Fix whitespace
>>    
>>    Reviewed-by: xpeng, kdnilsen
>>  - 8342927: GenShen: Guarantee slices of time for coalesce and filling
>>    
>>    Reviewed-by: kdnilsen
>>  - 8342924: GenShen: Problem list gc/shenandoah/TestReferenceRefersToShenandoah.java
>>    
>>    Reviewed-by: kdnilsen, ysr
>>  - 8342848: Shenandoah: Marking bitmap may not be completely cleared in generational mode
>>    
>>    Reviewed-by: wkemper
>>  - ... and 493 more: https://git.openjdk.org/jdk/compare/1c448347...19b25bc3
>
> src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp line 238:
> 
>> 236: #define shenandoah_assert_control_or_vm_thread_at_safepoint()
>> 237: #define shenandoah_assert_generational()
>> 238: #define shenandoah_assert_generations_reconciled()                                                             \
> 
> There seems to be dangling `` on this line.

Good catch.

> src/hotspot/share/gc/shenandoah/shenandoahStackWatermark.cpp line 77:
> 
>> 75:     return reinterpret_cast<OopClosure*>(context);
>> 76:   } else {
>> 77:     if (_heap->is_concurrent_weak_root_in_progress()) {
> 
> Comprehension check: We flips this because in generational mode we _can_ have both concurrent weak roots and concurrent mark in progress at the same time, and we need to prioritize evacs, right?

Correct. Although no old marking threads will ever run during a young collection.

> src/hotspot/share/gc/shenandoah/shenandoahUtils.hpp line 50:
> 
>> 48:   switch (generation_type) {                                              \
>> 49:     case NON_GEN:                                                         \
>> 50:       return prefix " (NON-GENERATIONAL)" postfix;                        \
> 
> In the interest of keeping the non-generational Shenandoah logging intact, should we drop ` (NON-GENERATIONAL)` here?

No objections here. @ysramakrishna , @kdnilsen ?

> src/hotspot/share/gc/shenandoah/shenandoahVMOperations.cpp line 83:
> 
>> 81: void VM_ShenandoahInitMark::doit() {
>> 82:   ShenandoahGCPauseMark mark(_gc_id, "Init Mark", SvcGCMarker::CONCURRENT);
>> 83:   set_active_generation();
> 
> Why is it here, and not down in `entry_*` methods, probably in helper classes?

I think it's here for the same reason `propagate_gc_state_to_java_threads` is here. Having it here makes it easier to see that this critical synchronization step happens for every safepoint.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1841308265
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1841308915
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1841310699
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1841313192