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

[Roman Kennke]

On Tue, 8 Oct 2024 17:20:31 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> This PR merges JEP 404, a generational mode for the Shenandoah garbage collector. The JEP can be viewed here: https://openjdk.org/jeps/404. We would like to target JDK24 with this PR.
>
> William Kemper has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 478 commits:
> 
>  - Fix merge error
>  - Merge remote-tracking branch 'jdk/master' into great-genshen-pr-redux
>  - Merge remote-tracking branch 'jdk/master' into great-genshen-pr-redux
>  - Merge branch 'shenandoah/master' into great-genshen-pr-redux
>  - Merge
>  - 8341099: GenShen: assert(HAS_FWD == _heap->has_forwarded_objects()) failed: Forwarded object status is sane
>    
>    Reviewed-by: kdnilsen
>  - 8341485: GenShen: Make evac tracker a non-product feature and confine it to generational mode
>    
>    Reviewed-by: kdnilsen, ysr
>  - Merge
>  - 8341042: GenShen: Reset mark bitmaps for unaffiliated regions when preparing for a cycle
>    
>    Reviewed-by: kdnilsen
>  - 8339616: GenShen: Introduce new state to distinguish promote-in-place phase as distinct from concurrent evacuation
>    
>    Reviewed-by: kdnilsen, shade, ysr
>  - ... and 468 more: https://git.openjdk.org/jdk/compare/b9db74a6...4db1e0e1

That is a big change - great work!
In this first batch I've reviewed:


src/hotspot/cpu
src/hotspot/gc/shared
src/hotspot/gc/shenandoah/c1
src/hotspot/gc/shenandoah/c2
src/hotspot/gc/shenandoah/heuristics
src/hotspot/gc/shenandoah/mode


I only have a few comments so far. Will review the remaining parts in separate batches.

src/hotspot/share/gc/shenandoah/c1/shenandoahBarrierSetC1.cpp line 75:

> 73: 
> 74:   // Create a mask to test if the marking bit is set.
> 75:   // TODO: can we directly test if bit is set?

That comment here is quite justified, IMO. I'm pretty sure that we could test for the flag in a single instruction, instead of doing the and-cmp sequence and even allocating a new register. Unless of course when C1 LIR can't represent it. Have you tried to implement that and failed, and therefore remove the comment?

src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.cpp line 96:

> 94:   size_t capacity    = _space_info->soft_max_capacity();
> 95:   size_t max_cset    = (size_t)((1.0 * capacity / 100 * ShenandoahEvacReserve) / ShenandoahEvacWaste);
> 96:   size_t free_target = (capacity * ShenandoahMinFreeThreshold) / 100 + max_cset;

Does re-arranging the math here risk overflow (e.g. on 32-bit)?

src/hotspot/share/gc/shenandoah/heuristics/shenandoahHeuristics.hpp line 79:

> 77: 
> 78: protected:
> 79:   static const uint Moving_Average_Samples = 10; // Number of samples to store in moving averages

I've never seen that style for constants in HotSpot before. I'd either make it MOVING_AVERAGE_SAMPLES or MovingAverageSamples. I think the latter is more prevalent, but I am not certain.

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

Changes requested by rkennke (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21273#pullrequestreview-2360208858
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1795359413
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1795377554
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1795401675