RFR(S) [15] : 8249000 : vm.gc.X should take selected JIT into account

[Vladimir Kozlov]

Good.

Thanks,
Vladimir

On 7/8/20 11:40 AM, Igor Ignatyev wrote:
> Thanks Vladimir.
> 
> for the record, I've updated ProblemList-graal.txt w/ the following:
> 
>> diff -r 14ffd658a23a test/hotspot/jtreg/ProblemList-graal.txt
>> --- a/test/hotspot/jtreg/ProblemList-graal.txt  Wed Jul 08 11:35:30 2020 -0700
>> +++ b/test/hotspot/jtreg/ProblemList-graal.txt  Wed Jul 08 11:37:44 2020 -0700
>> @@ -229,6 +229,7 @@
>>   compiler/loopopts/TestOverunrolling.java                8207267   generic-all
>>   compiler/jsr292/NonInlinedCall/InvokeTest.java          8207267   generic-all
>>   compiler/codegen/TestTrichotomyExpressions.java         8207267   generic-all
>> +gc/stress/TestReclaimStringsLeaksMemory.java            8207267   generic-all
>>   
>>   runtime/exceptionMsgs/AbstractMethodError/AbstractMethodErrorTest.java        8222582 generic-all
> 
> -- Igor
> 
> 
>> On Jul 8, 2020, at 11:34 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>
>> Thank you, Igor
>>
>> I got the difference between `vm.gc` and `vm.gc.X`.
>>
>> In this case TestReclaimStringsLeaksMemory.java should be put into ProblemList-graal.txt with 8207267 to enable it with libgraal.  Current usage of !vm.graal.enabled in test is to skip this test with Java Graal because its effect on Java heap.
>>
>> On 7/7/20 8:30 PM, Igor Ignatyev wrote:
>>> Hi Vladimir,
>>> thanks for your review!
>>> `vm.gc` and `vm.gc.X`-s are different beasts (and admittedly, they confuse people a lot), `vm.gc` is set to "X", by jtreg itself, only if there is UseXGC in vm flags, otherwise it's "null". `vm.gc.X` are set by VMProps class, and you can have more than one vm.gc.X == true, as vm.gc.X means that X gc is supported by JVM and it can be selected; so if there are no Use.*GC in vm flags, vm.gc.X will yield true for all GCs which JVM was built with; if one of UseXGC is provided, only corresponding vm.gc.X is true, and all others are false. so to answer your questions, yes `vm.gc` can be "null" (if there are no Use.*GC) , and yes `vm.gc.Z & vm.gc.Serial & vm.gc == null` can be true (if there are no Use.*GC and JVM supports both Z and Serial GCs).
>>
>> Interesting. I thought vmGC will list only one selected GC. That explains requires in TestZGCWithCDS.java.
>>
>> You only need to add TestReclaimStringsLeaksMemory.java into ProblemList-graal.txt.
>>
>> Thanks,
>> Vladimir
>>
>>> Thanks,
>>> -- Igor
>>>> On Jul 7, 2020, at 8:00 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>>
>>>> Nice clean up, Igor
>>>>
>>>> test/hotspot/jtreg/gc/stress/TestReclaimStringsLeaksMemory.java
>>>>
>>>> Do we even can have vm.gc=="null" based on code in VMProps.java? At least some GC should be selected ergonomically even if non is specified on command line.
>>>>
>>>> - * @requires vm.gc=="null" & !vm.graal.enabled & !vm.debug
>>>> + * @requires vm.gc == "null"
>>>> + * @requires !vm.debug
>>>>
>>>>
>>>> test/hotspot/jtreg/runtime/cds/appcds/TestZGCWithCDS.java
>>>>
>>>> Does next combination of @requires ever work? I thought such sequence means 'AND' operation on all such conditions.
>>>>
>>>>   * @requires vm.gc.Z
>>>>   * @requires vm.gc.Serial
>>>>   * @requires vm.gc == null
>>>>
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 7/7/20 5:38 PM, Igor Ignatyev wrote:
>>>>> http://cr.openjdk.java.net/~iignatyev/8249000/webrev.00/
>>>>>> 241 lines changed: 34 ins; 5 del; 202 mod;
>>>>> Hi all,
>>>>> could you please review the patch which modifies requires/VMProps to set vm.gc.X to false if Graal is selected and X GC isn't supported by Graal?
>>>>> the patch also replaces @requires similar to `vm.gc.X & !vm.graal.enabled` w/ `vm.gc.X` where it's applicable.
>>>>> from JBS:
>>>>>> not all GCs are supported by Graal JIT, which leads to failures like JDK-8247527 and boilerplate fixes like replacing all `@requires vm.gc.Z` w/ `@requires vm.gc.Z & !vm.graal.enabled`.
>>>>>>
>>>>>> as vm.gc.X means that X GC can be selected, it would be more natural, less surprising, and much more clear to have it true if the selected JIT supports the said X GC.
>>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8249000/webrev.00/
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8249000
>>>>> testing: test/hotspot/jtreg/{gc,compiler,runtime,serviceability} on {linux,windows,macos}-x64 w/ and w/o Graal as JIT
>>>>> Thanks,
>>>>> -- Igor
>