RFR: Restrict generational mode to adaptive heuristic

[Aleksey Shipilev]

On Fri, 24 Mar 2023 18:59:50 GMT, William Kemper <wkemper at openjdk.org> wrote:

> This is a small change to Shenandoah, but it prevents us from running the tests with the generational mode enabled via a jtreg vm option. This requires us to properly integrate the generational mode with the jtreg tests. There are many test files changed, but the `generational` mode is now handled in the same fashion as other modes (`iu`, `passive`). There are two additional advantages to these changes:
> * We no longer need to run the test suite twice (once in the default mode and again with `-XX:ShenandoahGCMode=generational`).
> * Expanded test coverage, as some tests would spawn their own VMs with permutations of mode and heuristic options that ignored the generational mode.

Questions and suggestions below:

test/hotspot/jtreg/gc/shenandoah/TestAllocHumongousFragment.java line 61:

> 59:  *
> 60:  * @run main/othervm -Xmx1g -Xms1g -Xlog:gc -XX:+UnlockDiagnosticVMOptions -XX:+UnlockExperimentalVMOptions -XX:ShenandoahTargetNumRegions=2048
> 61:  *      -XX:+UseShenandoahGC -XX:ShenandoahGCHeuristics=aggressive -XX:ShenandoahGCMode=satb

Why add `satb` in this test, but not the others? Other tests seem to just run `aggressive` with default GC mode.

test/hotspot/jtreg/gc/shenandoah/TestAllocObjectArrays.java line 121:

> 119:  *      -XX:+UseShenandoahGC -XX:ShenandoahGCHeuristics=adaptive -XX:ShenandoahGCMode=generational
> 120:  *      -XX:+ShenandoahOOMDuringEvacALot -XX:+ShenandoahVerify
> 121:  *      TestAllocObjectArrays

Please put `-XX:+ShenandoahVerify` on the separate line, like the test config above?

test/hotspot/jtreg/gc/shenandoah/TestAllocObjectArrays.java line 167:

> 165:  * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:+UnlockExperimentalVMOptions -Xmx1g -Xms1g
> 166:  *      -XX:+UseShenandoahGC -XX:ShenandoahGCHeuristics=adaptive -XX:ShenandoahGCMode=generational
> 167:  *      -XX:-UseTLAB -XX:+ShenandoahVerify

This `-XX:+ShenandoahVerify` seems excessive. This config is supposed to run without verification. Copy-paste error, it seems.

test/hotspot/jtreg/gc/shenandoah/TestParallelRefprocSanity.java line 1:

> 1: /*

Why is this removed?

test/hotspot/jtreg/gc/shenandoah/TestRegionSamplingLogging.java line 31:

> 29:  * @run main/othervm -Xmx1g -Xms1g -XX:+UnlockDiagnosticVMOptions -XX:+UnlockExperimentalVMOptions
> 30:  *      -XX:+ShenandoahRegionSampling -XX:+ShenandoahRegionSampling
> 31:  *      -Xlog:gc+region=trace:region-snapshots-%p.log::filesize=100,filecount=3

So, `trace` is enough here? This looks like something we need to fix upstream, rather than do in a bulk change like this.

test/hotspot/jtreg/gc/shenandoah/TestRetainObjects.java line 107:

> 105:  * @run main/othervm -Xmx1g -Xms1g -XX:+UnlockDiagnosticVMOptions -XX:+UnlockExperimentalVMOptions
> 106:  *      -XX:+UseShenandoahGC -XX:ShenandoahGCHeuristics=adaptive -XX:ShenandoahGCMode=generational
> 107:  *      -XX:+ShenandoahAllocFailureALot -XX:+ShenandoahVerify

I think the intent here is to run without `-XX:+ShenandoahVerify`.

test/hotspot/jtreg/gc/shenandoah/TestSieveObjects.java line 107:

> 105:  * @run main/othervm -Xmx1g -Xms1g -XX:+UnlockDiagnosticVMOptions -XX:+UnlockExperimentalVMOptions
> 106:  *      -XX:+UseShenandoahGC -XX:ShenandoahGCHeuristics=adaptive -XX:ShenandoahGCMode=generational
> 107:  *      -XX:+ShenandoahAllocFailureALot -XX:+ShenandoahVerify

Excessive `-XX:+ShenandoahVerify`?

test/hotspot/jtreg/gc/shenandoah/compiler/BarrierInInfiniteLoop.java line 35:

> 33:  */
> 34: 
> 35:  /**

This looks like a compiler regression test, so we don't need to run it in all these different GC modes.

test/hotspot/jtreg/gc/shenandoah/compiler/CallMultipleCatchProjs.java line 34:

> 32:  */
> 33: 
> 34:  /**

Again, compiler regression test, probably no reason to run with different GC modes.

test/hotspot/jtreg/gc/shenandoah/compiler/FoldIfAfterExpansion.java line 36:

> 34:  */
> 35: 
> 36: /**

Again, compiler regression test, probably no reason to run with different GC modes.

test/hotspot/jtreg/gc/shenandoah/compiler/LRBRightAfterMemBar.java line 35:

> 33:  */
> 34: 
> 35:  /**

Right, `compiler` test again. :) I think most (all?) tests in this folder do not need GC mode variance.

test/hotspot/jtreg/gc/shenandoah/compiler/TestClone.java line 208:

> 206:  */
> 207: 
> 208: /*

This one seems to be the rare exception where GC mode variance is meaningful -- this is not a targeted regression test.

test/hotspot/jtreg/gc/shenandoah/generational/TestCLIModeGenerational.java line 34:

> 32:  * @library /testlibrary /test/lib /
> 33:  * @build jdk.test.whitebox.WhiteBox
> 34:  * @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox

Generally, these changes should be done separately, not to clobber the actual "generational"-related test change. It is fine to do them here, if these only touch the files than exist only in GenShen.

test/hotspot/jtreg/gc/shenandoah/oom/TestAllocOutOfMemory.java line 35:

> 33: /**
> 34:  * @test id=heap
> 35:  * @summary Test allocation of small object to result OOM, but not to crash JVM

The `@summary` in these configs are all the same, copy-paste error.
Also, we might want to fix English here a bit:
`Test allocation of (small|large|larger-than-heap) object should result in OOM, not in JVM crash`.

test/hotspot/jtreg/gc/stress/jfr/TestStressBigAllocationGCEventsWithShenandoah.java line 1:

> 1: /*

Is there a generational variant for this?

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

PR Review: https://git.openjdk.org/shenandoah/pull/233#pullrequestreview-1358736166
PR Review Comment: https://git.openjdk.org/shenandoah/pull/233#discussion_r1149058740
PR Review Comment: https://git.openjdk.org/shenandoah/pull/233#discussion_r1149060202
PR Review Comment: https://git.openjdk.org/shenandoah/pull/233#discussion_r1149061221
PR Review Comment: https://git.openjdk.org/shenandoah/pull/233#discussion_r1149065439
PR Review Comment: https://git.openjdk.org/shenandoah/pull/233#discussion_r1149070147
PR Review Comment: https://git.openjdk.org/shenandoah/pull/233#discussion_r1149070925
PR Review Comment: https://git.openjdk.org/shenandoah/pull/233#discussion_r1149071593
PR Review Comment: https://git.openjdk.org/shenandoah/pull/233#discussion_r1149073540
PR Review Comment: https://git.openjdk.org/shenandoah/pull/233#discussion_r1149074305
PR Review Comment: https://git.openjdk.org/shenandoah/pull/233#discussion_r1149074544
PR Review Comment: https://git.openjdk.org/shenandoah/pull/233#discussion_r1149075454
PR Review Comment: https://git.openjdk.org/shenandoah/pull/233#discussion_r1149076687
PR Review Comment: https://git.openjdk.org/shenandoah/pull/233#discussion_r1149077952
PR Review Comment: https://git.openjdk.org/shenandoah/pull/233#discussion_r1149086518
PR Review Comment: https://git.openjdk.org/shenandoah/pull/233#discussion_r1149089962