RFR: 8311978: Create abstraction over heap metrics for heuristics [v3]

[Y. Srinivas Ramakrishna]

On Thu, 13 Jul 2023 23:54:33 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> This change introduces a new interface `ShenandoahHeapStats`. Presently, the interface is implemented by `ShenandoahHeap`. The generational mode for Shenandoah will provide a new implementation of this interface `ShenandoahGeneration`.
>
> William Kemper has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Put explicit constructor call back in

This looks good to me, and I'd be happy to approve.

However, I'd like us to first review the HotSpot style guide to see what its stance is on the use of multiple inheritance where all but one parent class are pure virtual, and to obtain review & approval from someone familiar with the current stance on this.

src/hotspot/share/gc/shenandoah/mode/shenandoahSATBMode.cpp line 66:

> 64:     return new ShenandoahAdaptiveHeuristics(ShenandoahHeap::heap());
> 65:   } else if (strcmp(ShenandoahGCHeuristics, "compact") == 0) {
> 66:     return new ShenandoahCompactHeuristics();

It feels a bit ad-hoc to me that ShenandoahStaticHeuristic & ShenandoahStaticHeuristic, which both use some information about the state of the heap (i.e. state queriable through ShenandoahHeapStats interface methods), don't get passed that argument in the constructor. I'd ideally like to see the c'tors changed to hew to the same pattern rather than their reaching directly into the global heap object to get what they need.

A question to ask is, if a new heuristic were to be designed in the future, would it extend ShenandahHeapStats, being the official interface through which heuristics make use of properties of the heap upon which they operate to extract that information?

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

PR Review: https://git.openjdk.org/jdk/pull/14856#pullrequestreview-1529428527
PR Review Comment: https://git.openjdk.org/jdk/pull/14856#discussion_r1263179127