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

[Stefan Karlsson]

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 can change the name, but I think that would come with a maintenance 
cost. What do you think?

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