RFR: 8311602 GenShen: Decouple generational mode heuristics [v3]

[Y. Srinivas Ramakrishna]

On Tue, 11 Jul 2023 00:51:06 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> The general idea here is to straighten out code paths which fork based on the mode configuration. A `ShenandoahYoungHeuristics` class has been added to host the logic specific to the generational mode. In many cases, the entangled generational code can simply be moved behind methods which override existing virtual methods. 
>> 
>> A few other notable changes:
>> * The confusing "trigger" heuristic has been removed from `ShenandoahOldHeuristics`.
>>   * The old heuristic defines its own triggers now. It is no longer possible to specify the old generation's trigger heuristic.
>> * Selection of aged regions for in-place promotion has been moved to `ShenandoahGeneration`.
>>   * The method implementation does not depend on any members of `ShenandoahHeuristics`. Moving the method therefore removes an unnecessary dependency on the heuristics.
>> * A new `ShenandoahHeapCharacteristics` interface has been added to provide information about the heap _or generation_ to the heuristics.
>>   * On the development branch, this interface is implemented by `ShenandoahGeneration`.
>>   * The plan is to upstream this interface and have it implemented by `ShenandoahHeap` 
>> 
>> The taxonomy of these heuristic classes has become somewhat more complex - here is a chart showing the relationships:
>> 
>> 
>> ShenandoahHeuristics
>> -  ShenandoahPassiveHeuristics
>> -  ShenandoahCompactHeuristics
>> -  ShenandoahAggressiveHeuristics
>> -  ShenandoahStaticHeuristics
>> -  ShenandoahOldHeuristics
>> +  ShenandoahAdaptiveHeuristics
>>   +  ShenandoahGenerationalHeuristics
>>     -  ShenandoahYoungHeuristics
>>     -  ShenandoahGlobalHeuristics
>
> William Kemper has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - Use interface in heuristic initialization methods
>  - Call base class method with more intuitive syntax.
>  - Remove unused fields and methods

src/hotspot/share/gc/shenandoah/heuristics/shenandoahHeapCharacteristics.hpp line 30:

> 28: #include "utilities/globalDefinitions.hpp"
> 29: 
> 30: class ShenandoahHeapCharacteristics {

Please include a brief documentation comment of what it represents and what its intended use is.

src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp line 487:

> 485: // A second benefit of treating aged regions differently than other regions during collection set selection is
> 486: // that this allows us to more accurately budget memory to hold the results of evacuation.  Memory for evacuation
> 487: // of aged regions must be reserved in the old generations.  Memory for evacuation of all other regions must be

Memory for objects with age exceeding tenuring threshold could be in regions with age "0", correct? If that is so, the last sentence here may be slightly inaccurate. Or perhaps it will be inaccurate when adaptive tenuring is actually in place? (Although I'd imagine that it might be inaccurate even today with a fixed/static tenuring age.)

src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp line 490:

> 488: // reserved in the young generation.
> 489: //
> 490: // A side effect performed by this function is to tally up the number of regions and the number of live bytes

The side-effect and the return value should also be documented in the header file, because they seem to be part of the (implicit) API contract for the method?

src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp line 953:

> 951: void ShenandoahGeneration::record_success_concurrent(bool abbreviated) {
> 952:   ShenandoahHeap* heap = ShenandoahHeap::heap();
> 953:   abbreviated = abbreviated && heap->mode()->is_generational();

You'll need to explain this change. Why should it matter here if we are generational or not for an abbreviated collection to be counted as one. This is confusing to the lay reader like me :-)

src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp line 88:

> 86:   // Preselect for inclusion into the collection set regions whose age is
> 87:   // at or above tenure age which contain more than ShenandoahOldGarbageThreshold
> 88:   // amounts of garbage.

Document side-effect that other code relies on, as well as the return value (see comment in .cpp implementation of method, around line 490 in this version of the diff.)

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/292#discussion_r1261719161
PR Review Comment: https://git.openjdk.org/shenandoah/pull/292#discussion_r1261706301
PR Review Comment: https://git.openjdk.org/shenandoah/pull/292#discussion_r1261707760
PR Review Comment: https://git.openjdk.org/shenandoah/pull/292#discussion_r1261699477
PR Review Comment: https://git.openjdk.org/shenandoah/pull/292#discussion_r1261709611