[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR(XS): 8241009: CommandLineFlagComboNegative.java fails after JDK-8240563
From:       Yumin Qi <yumin.qi () oracle ! com>
Date:       2020-03-19 16:59:20
Message-ID: 0a903ae6-39d0-811d-5d9e-b8dc2ca6a18e () oracle ! com
[Download RAW message or body]

HI, Stefan

   Thanks. I upload the first version at: 
http://cr.openjdk.java.net/~minqi/8241009/webrev-00/
   if @requires condition failed, the test won't be selected and result 
won't be listed. With SkippedException, the result shows what reason the 
test skipped.

   Thanks
   Yumin


On 3/19/20 1:12 AM, Stefan Karlsson wrote:
> 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
>>>>>
>>>>
>>>
>

[prev in list] [next in list] [prev in thread] [next in thread]