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

[Kelvin Nilsen]

On Tue, 4 Feb 2025 15:50:59 GMT, Paul Hohensee <phh at openjdk.org> wrote:

>> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Respond to reviewer feedback
>>   
>>   In testing suggested refinements, I discovered a bug in original
>>   implementation.  ShenandoahFreeSet::capacity() does not represent the
>>   size of young generation.  It represents the total size of the young
>>   regions that had available memory at the time we most recently rebuilt
>>   the ShenandoahFreeSet.
>>   
>>   I am rerunning the performance tests following this suggested change.
>
> 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;
> 
> As an outsider, the units involved and what exactly is being calculated is pretty opaque. Why would we divide by 100 to compute free_expected and not do the same for free_actual? Do we care about integer division truncation? The default value of ShenandoahCriticalFreeThreshold is 1, so multiplying by it is a nop by default, which seems strange.

ShenandoahCriticalFreeThreshold represents a percentage of the "total size".  To calculate N% of the young generation size, we divide the generation size by 100 and then multiply by ShenandoahCriticalFreeThreshold.  This code is a bit different in the most recent revision.  Do you think it needs a comment?

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

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