RFR: 8358529: GenShen: Heuristics do not respond to changes in SoftMaxHeapSize [v2]

[Rui Li]

On Mon, 30 Jun 2025 23:02:23 GMT, Rui Li <duke at openjdk.org> wrote:

>> src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp line 1017:
>> 
>>> 1015: size_t ShenandoahGeneration::soft_max_capacity() const {
>>> 1016:   size_t capacity = ShenandoahGenerationalHeap::heap()->soft_max_capacity();
>>> 1017:   log_debug(gc)("soft_max_capacity: %zu", capacity); // TestDynamicSoftMaxHeapSize needs the log line to validate
>> 
>> This will be a hot method. It's called by the heuristic frequently to test if it should start a GC. We may also even test this on the allocation path. I'd rather not have a log level check here just for one test case. Can we move this log message to the place where the value is updated? I expect the value to change _much less_ frequently than it is read.
>
> Agree that the debug log can be too much in the log and impact perf. There is an existing log when soft max is changed: [link](https://github.com/openjdk/jdk/blob/6df0f5e390ecf874c1eca7284c51efa65ce23737/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp#L865-L868). The problem is, before the fix, when the soft max was changed, gen shen  doesn't read the changed value. That's why I put a log here. Validating log is also indeed a bit fragile. If we remove the log, probably need to think another way to test.
> 
> 
> For removing `soft_max_capacity`: not sure if I understand it right, but I thought `ShenandoahSpaceInfo` was the parent class in the polymorphism ([code](https://github.com/openjdk/jdk/blob/6df0f5e390ecf874c1eca7284c51efa65ce23737/src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.hpp#L68)), so the code would work for both gen shen (ShenandoahGeneration) and ShenandoahGlobalGeneration? If we removed `soft_max_capacity ` from ShenandoahSpaceInfo, wouldn't that make heuristics not able to access soft max heap size?

Realized just simply replace `_space_info->soft_max_capacity()` with `ShenandoahHeap::heap()->soft_max_capacity()`. 

Updated in the latest commit.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25943#discussion_r2188877998