RFR: 8327000: GenShen: Integrate updated Shenandoah implementation of FreeSet into GenShen [v8]

[Y. Srinivas Ramakrishna]

On Tue, 18 Jun 2024 00:41:09 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Minor refinements to test programs
>>   
>>   TestAllocIntArrays: comments to explain behavior.
>>   TestOldGrowthTriggers: reduce the number of loop iterations so this test
>>   will not time out on less powerful test platforms.
>
> src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 992:
> 
>> 990:     // gc_no_progress_count is incremented following each degen or full GC that fails to achieve is_good_progress().
>> 991:     // Note that Generational Shenandoah may increment no_progress_count faster than traditional Shenandoah because young
>> 992:     // GCs, which may degenerate, typically occur more frequently than single-generation Global GCs.
> 
> Should this documentation comment about the expected difference in rates of incrementation occur in the vicinity of any differences (or otherwise) in the setting of the default value of `ShenandoahNoProgressThreshold` in each case -- or if the default values are the same, then where the option is documented?
> 
> I also wonder if `is_good_progress()` and `gc_no_progress_count` should apply uniformly in both cases to whole heap collections rather than to young collections (in the generational case). Otherwise, don't we run the risk of declaring no progress prematurely without a global collection in the case of generational, thus breaking in spirit the requirement of "best efforts attempt to reclaim unused memory"?

In particular, how is the notion of progress treated in other generational collectors?

> test/hotspot/jtreg/gc/shenandoah/oom/TestThreadFailure.java line 93:
> 
>> 91:             ProcessBuilder pb = ProcessTools.createLimitedTestJavaProcessBuilder(
>> 92:                     "-Xmx32m",
>> 93:                     "-XX:+UnlockExperimentalVMOptions", "-XX:ShenandoahNoProgressThreshold=24",
> 
> See my previous comment about when `ShenandoahNoProgressThreshold` should increment earlier in code.

Hmm, interesting, it looks like this does indeed increment only at a full gc, or at least that's the intent. Should check that this is also respected in the generational case. Vide:


  product(uintx, ShenandoahNoProgressThreshold, 5, EXPERIMENTAL,            \
          "After this number of consecutive Full GCs fail to make "         \
          "progress, Shenandoah will raise out of memory errors. Note "     \
          "that progress is determined by ShenandoahCriticalFreeThreshold") \
                                                                            \

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/440#discussion_r1643587192
PR Review Comment: https://git.openjdk.org/shenandoah/pull/440#discussion_r1643622489