RFR: 8325673: GenShen: Share Reserves between Old and Young Collector [v2]

[William Kemper]

On Mon, 15 Jul 2024 14:29:43 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

>> Allow young-gen Collector reserve to share memory with old-gen Collector reserve in order to support prompt processing of mixed evacuations, as constrained by ShenandoahOldEvacRatioPercent.
>
> Kelvin Nilsen has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 70 commits:
> 
>  - Remove unreferenced local variable
>  - Fix whitespace
>  - Remove debug instrumentation and deprecated code
>  - Turn off instrumentation
>  - Verifier should only count non-trashed committed regions
>  - Ignore generation soft capacities when adjusting generation sizes
>    
>    Soft capacities are established, for example, by setting NewRatio or New size on the
>    JVM command line.  GenShen, for now at least, does not honor these settings.  Better
>    performance is obtained by allowing GenShen to expand and shrink generation sizes
>    according to application behavior.
>    
>    This commit also tidies up various aspects of the implementation to make adjustments
>    to generation sizing more consistent:
>    
>    1. ShenandoahGlobalHeuristics::choose_global_collection_set(): share the reserves
>       between young and old collection to maximize evacuation of garbage-first
>       regions, regardless of whether most garbage is found in old or young
>    2. ShenandoahConcurrentGC::entry_final_roots(): do not balance generations before
>       invoking finish_rebuild() because finish_rebuild will balance generations.
>    3. ShenandoahFreeSet::flip_to_old_gc(): invoke force_transfer_to_young() instead
>       of transfer_to_young() so we can override soft-capacity limits
>    4. ShenandoahFullGC::phase5_epilog(): Do not invoke compute_balances() or
>       balance_generations_after_rebuilding_free_set().  Allow the free-set
>       rebuild() implementation to do this work in a more consistent fashion.
>    5. ShenandoahGeneration::adjust_evacuation_budgets(): replace transfer_to_youn()
>       with force_transfer_to_young() to avoid enforcement of soft capacity limits.
>    6. ShenandoahGenerationSizer::force_transfer_to_young(): new method
>    7. ShenandoahGenerationalFullGC::balance_generations_after_gc(): establish
>       reserves() so that free-set rebuild() can adjust balance.  Do not redundantly
>       force transfer of regions here.
>    8. ShenandoahGenerationalFullGC::balance_generations_after_rebuilding_free_set():
>       deprecate this method.
>    9. ShenandoahGenerationalFullGC::compute_balances(): deprecate this method.
>    10. ShenandoahGenerationaStatsClosure::validate_usage() (part of Shenandoah
>        Verification): add consistency check for generation capacities
>  - Fix budgeting error during freeset rebuild
>    
>    Limit the size of old-gen by memory av...

Changes requested by wkemper (Committer).

src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.cpp line 179:

> 177: }
> 178: 
> 179: void ShenandoahOldHeuristics::initialize_piggyback_evacs(ShenandoahCollectionSet* collection_set,

Can we continue using `mixed` instead of `piggyback` to describe collections that include young and old regions? I think `mixed` is an accepted term and `piggyback` feels a little bit too colloquial (also not sure if the phrase is commonly known outside of English).

src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.cpp line 185:

> 183:                                                          size_t &unfragmented_available,
> 184:                                                          size_t &fragmented_available,
> 185:                                                          size_t &excess_fragmented_available) {

Should all of these reference parameters be members? The API feels complicated now with these three methods passing the same parameters to each other:


initialize_piggyback_evacs(...)

prime_collection_set(...)

finalize_piggyback_evacs(...)


Could everything just happen within `prime_collection_set` without so many changes to exiting callers?

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

PR Review: https://git.openjdk.org/shenandoah/pull/395#pullrequestreview-2178189515
PR Review Comment: https://git.openjdk.org/shenandoah/pull/395#discussion_r1678109072
PR Review Comment: https://git.openjdk.org/shenandoah/pull/395#discussion_r1678113040