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

[Kelvin Nilsen]

On Mon, 10 Jul 2023 20:29:29 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

>> William Kemper has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Remove overzealous assert
>>  - Fix missing imports (windows build failure)
>
> src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.cpp line 58:
> 
>> 56: const double ShenandoahAdaptiveHeuristics::MAXIMUM_CONFIDENCE = 3.291; // 99.9%
>> 57: 
>> 58: ShenandoahAdaptiveHeuristics::ShenandoahAdaptiveHeuristics(ShenandoahHeapCharacteristics* heap_info) :
> 
> I'm assuming we'll issue a separate mainline patch that does slight mainline refactoring so we'll have less diffs when we integrate GenShen upstream.   I wonder if it would be easier to review this if we saw that refactoring first?
> 
> This gets us much closer to the original code, but still has the new heap_info argument to this constructor.

We don't have to see both PRs at the same time, nor necessarily sequence them as hinted above.  Just trying to clarify the details of this.

> src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.cpp line 544:
> 
>> 542: 
>> 543:   // Otherwise, defer to inherited heuristic for gc trigger.
>> 544:   return ShenandoahHeuristics::should_start_gc();
> 
> I'm puzzling over this.  May resolve for myself after reading more code.  Not clear to me how ShenandoahHeuristics knows that I'm asking about trigger for OLD vs YOUNG.

I think I understand this now.  It looks like a static method invocation because we named the type for the method.  But this is really an instance method invocation for the superclass method.  The "code clues" would work better for me if we wrote this super->should_start_gc(), but I guess is not C++.  can we write it this->ShenandoahHeuristics::should_start_gc().

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/292#discussion_r1258884868
PR Review Comment: https://git.openjdk.org/shenandoah/pull/292#discussion_r1258873861