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

[Kelvin Nilsen]

On Mon, 15 Jul 2024 16:36:38 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> 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 budget...
>
> 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?

Thanks.  Good suggestion.  Am pursuing this.  Looks much cleaner.

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/395#discussion_r1680161701