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

[William Kemper]

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

>> 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.

Yes, next step will be to upstream the interface introduced here in a separate change (without these generational changes). I could have done that first, but I wanted to get the interface right for the generational mode.

>> 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().

Yes, I can do that.

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/292#discussion_r1258979571
PR Review Comment: https://git.openjdk.org/shenandoah/pull/292#discussion_r1258979929