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

[David Holmes]

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,
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
> 
>