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

[Rui Li]

On Mon, 23 Jun 2025 22:06:15 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> Generational shenandoah currently doesn't pick up the changes of managed flag `SoftMaxHeapSize` when the app is running. This is because the value of `_soft_max_capacity` in `shenandoahGeneration` is never changed.
>> 
>> This change delegates the soft max heap size in `shenandoahGeneration` to `ShenandoahGenerationalHeap::heap()->soft_max_capacity()`, which does pick up the flag value changes.
>> 
>> Also, found `ShenandoahHeap:: initialize` uses `_num_regions * reg_size_bytes` rather than user input flag value. Updated to using actual flag value.
>
> src/hotspot/share/gc/shenandoah/heuristics/shenandoahCompactHeuristics.cpp line 60:
> 
>> 58:   size_t min_threshold = capacity / 100 * ShenandoahMinFreeThreshold;
>> 59: 
>> 60:   log_debug(gc)("should_start_gc? available: %zu, soft_max_capacity: %zu", available, capacity);
> 
> Can we remove these, or at least make them `log_develop_debug`?

Sure. Removed.

> 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);
> 
> Can we add a comment here saying that we need this message for the `TestDynamicSoftMaxHeapSize`? Also, this property will be read far more often than it will be written. Can we put the log message in the place where the value changes?

Will add a comment. 

There is actually an existing info level log for soft_max_heap change: [code](https://github.com/openjdk/jdk/blob/6df0f5e390ecf874c1eca7284c51efa65ce23737/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp#L865-L868). (Side note: we can't simply validate this info log line in the test, since gen shen didn't use the value initially)

> src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp line 129:
> 
>> 127:   virtual ShenandoahHeuristics* initialize_heuristics(ShenandoahMode* gc_mode);
>> 128: 
>> 129:   size_t soft_max_capacity() const override;
> 
> Can we take out the `_soft_max_capacity` member from `ShenandoahGeneration` now?

Yes. Good catch. Removed

> src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 101:
> 
>> 99: #include "utilities/globalDefinitions.hpp"
>> 100: #include "utilities/powerOfTwo.hpp"
>> 101: #include "logging/log.hpp"
> 
> Let's keep the includes alphabetized.

These two aren't used actually. Removed.

> test/hotspot/jtreg/gc/shenandoah/TestDynamicSoftMaxHeapSize.java line 68:
> 
>> 66: 
>> 67:             ProcessBuilder satbPb = ProcessTools.createLimitedTestJavaProcessBuilder(satbGCModeArgsNoDegeneratedGC);
>> 68:             (new OutputAnalyzer(satbPb.start())).shouldHaveExitValue(0);
> 
> I don't think we need parentheses around `new OutputAnalyzer(...)`.

Right. Removed.

> test/hotspot/jtreg/gc/shenandoah/TestDynamicSoftMaxHeapSize.java line 89:
> 
>> 87:         genShenGCModeArgs.add("-XX:ShenandoahGCMode=generational");
>> 88: 
>> 89:         for (String heuristic : HEURISTICS) {
> 
> The generational mode will quietly ignore other heuristics: https://bugs.openjdk.org/browse/JDK-8342640

Updated the test to only test adaptive.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25943#discussion_r2162660601
PR Review Comment: https://git.openjdk.org/jdk/pull/25943#discussion_r2162660681
PR Review Comment: https://git.openjdk.org/jdk/pull/25943#discussion_r2162663178
PR Review Comment: https://git.openjdk.org/jdk/pull/25943#discussion_r2162696482
PR Review Comment: https://git.openjdk.org/jdk/pull/25943#discussion_r2162661332
PR Review Comment: https://git.openjdk.org/jdk/pull/25943#discussion_r2162661911