RFR: 8203923: Add @requires feature to check flag values for the running JVM

[Stefan Karlsson]

On 2018-05-30 01:44, David Holmes wrote:
> Hi Stefan,
>
> On 30/05/2018 8:07 AM, Stefan Karlsson wrote:
>> Hi David,
>>
>> On 2018-05-29 23:53, David Holmes wrote:
>>> Hi Stefan,
>>>
>>> On 30/05/2018 12:21 AM, Stefan Karlsson wrote:
>>>> Hi all,
>>>>
>>>> Please review this patch to add @requires vm.opt.final.<vm flag 
>>>> name>, as a way to filter jtreg tests on the final value of flags 
>>>> in the test VM.
>>>>
>>>> http://cr.openjdk.java.net/~stefank/8203923/webrev.01/
>>>> https://bugs.openjdk.java.net/browse/JDK-8203923
>>>>
>>>> This patch is needed to allow tests to be filtered out when ZGC is 
>>>> run.
>>>>
>>>> ZGC doesn't support UseCompressedOops (and currently doesn't 
>>>> support ClassUnloading). Instead of adding @requires vm.gc != "Z" 
>>>> for tests that rely on ClassUnloading, I think it's nicer to write 
>>>> @requires vm.opt.final.ClassUnloading.
>>>>
>>>> This will filter out the test if any of the following conditions 
>>>> are met:
>>>> 1) The currently executing VM doesn't support ClassUnloading
>>>> 2) -XX:-ClassUnloading is passed to the test
>>>>
>>>> The second case can today be handled by @requires 
>>>> vm.opt.ClassUnloading == NULL | vm.opt.ClassUnloading == true, or 
>>>> maybe simpler @requires vm.opt.ClassUnloading != false, but if you 
>>>> need both @requires vm.opt.final.ClassUnloading should be used.
>>>
>>> I'm not sure about the use of vm.opt.final as the name here. IIUC 
>>> vm.opt is set by jtreg based on the vmoptions passed to jtreg for 
>>> running the tests. Whereas this is the final flag value as seen in 
>>> the running VM. And given we can only inspect the final value this 
>>> way, perhaps just vm.flag would be better?
>>
>> One reason why I chose to use the vm.opt.final prefix, instead of 
>> some other prefix (like vm.flag), is that jtreg "conveniently" 
>> accepts any property prefixed with vm.opt. If I change the prefix to 
>> vm.flag I need to add every single vm.flag to the requires.properties 
>> in all TEST.ROOT files of the test directories where these properties 
>> are used (E.g. test/hotspot/jtreg/TEST.ROOT).
>>
>> See isValidName in:
>> http://hg.openjdk.java.net/code-tools/jtreg/file/7b1496d2790e/src/share/classes/com/sun/javatest/regtest/config/RegressionContext.java#l109 
>
>
> I see. That is unfortunate. I think we need to discuss with Jon how to 
> make this more flexible. vm.flag seems much better (and more accurate) 
> than vm.opt...
>
>> I can change the name, but I think that would come with a maintenance 
>> cost. What do you think?
>
> We can live with current name for now and see how to improve this.

Thanks,
StefanK

>
> Thanks,
> David
>
>>>
>>>> The current solution requires the queried flags to be listed in 
>>>> vmOptFinalFlags. Maybe there's a way to get this to cover all flags 
>>>> automatically?
>>>
>>> Yes it is a shame that this has to be manually configured. I think 
>>> ideally this would need to be built into jtreg so that it ran a 
>>> dcmd(?) to get all final flag values and then populated the 
>>> properties. Though I suppose VMProps could also do that ... (The 
>>> crude way of doing this is via PrintFlagsFinal and parsing the 
>>> output, of course).
>>>
>>> Anyway other than my query on the naming, this seems fine for moving 
>>> ahead.
>>
>> Thanks,
>> StefanK
>>
>>>
>>> Thanks,
>>> David
>>>
>>>> Thanks,
>>>> StefanK
>>
>>