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

[Kim Barrett]

On Wed, 30 Jun 2021 10:10:55 GMT, Per Liden <pliden at openjdk.org> wrote:

>> 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?
>
> Sorry, let me be more specific. I was proposing you do this (in all these tests):
> 
> ```diff --git a/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationFullGC.java b/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationFullGC.java
> index 66d58a6e580..6a358ae54f3 100644
> --- a/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationFullGC.java
> +++ b/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationFullGC.java
> @@ -27,26 +27,26 @@ package gc.stringdedup;
>   * @test TestStringDeduplicationFullGC
>   * @summary Test string deduplication during full GC
>   * @bug 8029075
> - * @requires vm.gc == "G1" | vm.gc == "Shenandoah"
> + * @requires vm.gc.G1
>   * @library /test/lib
>   * @library /
>   * @modules java.base/jdk.internal.misc:open
>   * @modules java.base/java.lang:open
>   *          java.management
> - * @run driver gc.stringdedup.TestStringDeduplicationFullGC
> + * @run driver gc.stringdedup.TestStringDeduplicationFullGC G1
>   */
>  
>  /*
>   * @test TestStringDeduplicationFullGC
>   * @summary Test string deduplication during full GC
>   * @bug 8029075
> - * @requires vm.gc == "null" & vm.gc.G1
> + * @requires vm.gc.Shenandoah
>   * @library /test/lib
>   * @library /
>   * @modules java.base/jdk.internal.misc:open
>   * @modules java.base/java.lang:open
>   *          java.management
> - * @run driver gc.stringdedup.TestStringDeduplicationFullGC G1
> + * @run driver gc.stringdedup.TestStringDeduplicationFullGC Shenandoah
>   */
>  
>  public class TestStringDeduplicationFullGC {

That's an approach that I had looked at but decided not to take.

It involves having a near duplicated `@test` block for each GC that supports
deduplication, at least until they all do (other than Epsilon).

I think it wastes testing resources. Each testing configuration that allows
ergonomic GC selection will run these tests for every supporting and
included GC.  Individually it's not terrible, but this sort of thing adds up.

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

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