RFR: 8348595: GenShen: Fix generational free-memory no-progress check [v2]

[Xiaolong Peng]

On Fri, 7 Feb 2025 23:54:46 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

>> src/hotspot/share/gc/shenandoah/shenandoahMetrics.cpp line 52:
>> 
>>> 50:   size_t free_actual   = free_set->available();
>>> 51:   // The sum of free_set->capacity() and ->reserved represents capacity of young in generational, heap in non-generational.
>>> 52:   size_t free_expected = ((free_set->capacity() + free_set->reserved()) / 100) * ShenandoahCriticalFreeThreshold;
>> 
>> We may pass ShenandoahGeneration as parameter to `is_good_progress` to simplify the calculation of free_expected, it should be like:
>> `
>> generation->max_capacity() / 100 * ShenandoahCriticalFreeThreshold
>> `
>> Good part is, free_expected might be more accurate in Full GC/Degen for global cycle, e.g. Full GC collects memory for global, `free_expected` should be calculated using the metrics from global generation. But either way, `free_expected` is not clearly defined in generational mode now, current code also works.
>
> Thanks for this suggestion.  I've made change.  It turns out there was actually a bug in the original implementation, so I am retesting the performance results.

Thanks, honest I didn't understand that why `(free_set->capacity() + free_set->reserved()` represents capacity of young in generational, is it the bug you found? `free_set->capacity()` excludes the regions doesn't have enough capacity(it is calculated when rebuild free set)

I thought a bit more, it makes more sense to calculate free_expected in `snap_before`, max_capacity of generations may change after collection, the free_expected should be calculated before the cycle.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23306#discussion_r1947405475