RFR(XS): 8241009: CommandLineFlagComboNegative.java fails after JDK-8240563

[Stefan Karlsson]

On 2020-03-18 23:54, David Holmes wrote:
> Hi Yumin,
>
> On 19/03/2020 7:43 am, Yumin Qi wrote:
>> Hi, Stefan and all
>>
>>    (Sorry last email spell your last name first).
>>    Update using your suggestion. Also remove the checking for 64bit 
>> code since after the change, checking against 64bit no longer needed.
>>
>> http://cr.openjdk.java.net/~minqi/8241009/webrev/
>
> This doesn't seem ideal but it does seem sufficient.
>
> Ideally the test would explicitly set all the flags it depends upon 
> rather than assuming/requiring the flags have a specific value.

Hmm. Yes, I might have misunderstood the reason why the filtering was 
done, and maybe agree with David here. I was trying to look at the 
original webrev again, but it has been overwritten with the new version. 
Could you add back the original version, and put the updated webrev in a 
separate directory?

I'm also fine with leaving this as is if we think this is good enough 
for now.

Thanks,
StefanK

>
> Thanks,
> David
>
>> Thanks
>> Yumin
>>
>> On 3/18/20 10:21 AM, Yumin Qi wrote:
>>> Hi, Karlsson
>>>   Thanks. that is more concise version. Will update.
>>>
>>> Yumin
>>>
>>> On 3/18/20 1:58 AM, Stefan Karlsson wrote:
>>>> Hi Yumin,
>>>>
>>>> On 2020-03-18 00:01, Yumin Qi wrote:
>>>>> Hi, please review
>>>>>
>>>>>   Bug:https://bugs.openjdk.java.net/browse/JDK-8241009 
>>>>> <https://bugs.openjdk.java.net/browse/JDK-8241009>
>>>>>   Webrev: http://cr.openjdk.java.net/~minqi/8241009/webrev/
>>>>>
>>>>>   The vmoptions from jtreg still affect the launched processes and 
>>>>> mess up the parameters for launched java processes in the test.
>>>>>   Fix skip the test if -XX:-UseCompressedOops or 
>>>>> -XX:-UseCompressedClassPointers in jtreg vmoptions (or javaoptions).
>>>>
>>>> We already have a way to use @require to disable tests when 
>>>> incompatible flags are passed.
>>>>
>>>> Take a look at:
>>>> test/hotspot/jtreg/runtime/CompressedOops/CompressedClassSpaceSize.java 
>>>>
>>>>
>>>>  * @requires vm.bits == 64 & vm.opt.final.UseCompressedOops == true
>>>>
>>>> Could this be used instead? Note that this is using 
>>>> vm.opt.final.UseCompressedOops and not vm.opt.UseCompressedOops. 
>>>> The former actually checks the value of the flag, just like your 
>>>> adhoc version does.
>>>>
>>>> The list of supported flags are currently hard-coded, so if you 
>>>> need a flag that's not already exposed, you'll have to add it 
>>>> yourself. See: test/jtreg-ext/requires/VMProps.java
>>>>
>>>>     /**
>>>>      * Selected final flag.
>>>>      *
>>>>      * @param map - property-value pairs
>>>>      * @param flagName - flag name
>>>>      */
>>>>     private void vmOptFinalFlag(SafeMap map, String flagName) {
>>>>         map.put("vm.opt.final." + flagName,
>>>>                 () -> String.valueOf(WB.getBooleanVMFlag(flagName)));
>>>>     }
>>>>
>>>>     /**
>>>>      * Selected sets of final flags.
>>>>      *
>>>>      * @param map - property-value pairs
>>>>      */
>>>>     protected void vmOptFinalFlags(SafeMap map) {
>>>>         vmOptFinalFlag(map, "ClassUnloading");
>>>>         vmOptFinalFlag(map, "ClassUnloadingWithConcurrentMark");
>>>>         vmOptFinalFlag(map, "UseCompressedOops");
>>>>         vmOptFinalFlag(map, "EnableJVMCI");
>>>>         vmOptFinalFlag(map, "EliminateAllocations");
>>>>     }
>>>>
>>>> Thanks,
>>>> StefanK
>>>>
>>>>>
>>>>>   Tests: tested with explicitly set -XX:-UseCompressedOops or 
>>>>> -XX:-UseCompressedClassPointers or both in jtreg vmoptions.
>>>>>   tier1,2,4,7 test pending.
>>>>>
>>>>> Thanks
>>>>> Yumin
>>>>
>>>
>>