RFR: 8269032: Stringdedup tests are failing if the ergonomically select GC does not support it

[Kim Barrett]

On Tue, 29 Jun 2021 11:02:56 GMT, Per Liden <pliden at openjdk.org> wrote:

>> Please review this change to the string deduplication tests' handling of an
>> ergonomically chosen GC.  They were assuming G1 would be chosen in that
>> case, but that's wrong of course.  The test machine might not be a server
>> class machine, or G1 might not be included in the build.
>> 
>> Each test now has a second test declaration comment for handling the case
>> where no GC is specified by the jtreg invocation.  This second declaration
>> will force the use of G1 by the test if G1 is supported by the VM.
>> 
>> I looked into trying to be more clever and selecting a different GC if G1 is
>> not supported by the VM, but that ended up making the tests a lot more
>> messy, and doesn't seem like that important a use-case at this time.  A
>> better long-term solution would be to make all the GCs (except Epsilon)
>> support string deduplication, so we don't care which GC gets ergonomically
>> chosen.  But that's not happening today.
>> 
>> Testing:
>> Ran the string deduplication tests with various configurations: (1)
>> explicitly use G1 (2) no explicit GC, (3) no explicit GC with
>> -XX:+NeverActAsServerClassMachine.
>
> test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationAgeThreshold.java line 44:
> 
>> 42:  * @bug 8029075
>> 43:  * @requires vm.gc == "null" & vm.gc.G1
>> 44:  * @library /test/lib
> 
> ... and same here, this should be `@requires vm.gc.Shenandoah` and it should pass `-XX:+UseShenandoahGC`to the test, which it appends to the list of arguments for the JVM that will be spawned.
> 
> Of course, same comments apply to all tests.

I think the eventual goal should probably be that these are all driver
tests, and just have `@requires vm.gc != "Epsilon"` with a "driver" test,
but we're not there yet.

Until then, what I'm attempting to do is get good test coverage without
wasting a lot of testing resources. Driver tests are preferred to othervm
tests where that's possible, for better resource utilization. Driver tests
can't use VM options.

Most of the string deduplication implementation is entirely GC-agnostic. So
ideally we would run these tests with a given GC as part of focused testing
of that GC, and not when testing some other GC. Hence the use of `@requires`
for specific GC options (vm.gc == "XXX").

But that doesn't provide any testing for the case where the GC is being
selected ergonomically. The new `@test` clauses to test with G1 are not
entirely ideal, since they won't run if someone builds a VM that excludes G1
but does include some other GC that supports string deduplication. But I
couldn't come up with a way to address that without having to deal with
WhiteBox.

I'm not sure exactly what you are proposing and how it might relate to the
above considerations.  I'm not enamored of what I've proposed, but haven't
come up with something I like better.  Maybe if you could be more explicit
about the diffs you think should be made for one of the tests?

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

PR: https://git.openjdk.java.net/jdk/pull/4603