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

[Rui Li]

On Mon, 30 Jun 2025 19:12:32 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> Rui Li has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   No need atomic load in initialization
>
> 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?

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

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