RFR: 8160088: update hotspot tests depending on GC to use @requires vm.gc.X

[David Holmes]

Hi Dmitry,

On 23/06/2016 6:52 PM, Dmitry Fazunenko wrote:
> Hi David,
>
> thanks for looking!
>
> On 23.06.2016 3:17, David Holmes wrote:
>> Hi Dmitry,
>>
>> On 23/06/2016 4:06 AM, Dmitry Fazunenko wrote:
>>> Hello,
>>>
>>> I'm looking for Reviewers for relatively simple fix which affects 59
>>> tests.
>>>   https://bugs.openjdk.java.net/browse/JDK-8160088
>>>   http://cr.openjdk.java.net/~dfazunen/8160088/webrev.00/
>>>
>>> The fix allows to skip execution of tests requiring a specific GC in
>>> case of the required GC is not supported by VM.
>>> Old variant:
>>>   @requires vm.gc == null | vm.gc == "G1"
>>>      A test will be executed if no GC is specified externally or
>>> -XX:+UseG1GC flag is given.
>>>      This test will be executed even if VM doesn't support G1 and fail.
>>> New variant:
>>>   @requires vm.gc.G1
>>>      This test will not be executed if VM doesn't support G1.
>>
>> That doesn't seem sufficient. What you want is that if the test
>> requires G1 then it also supports G1, so it seems to me the correct
>> formulation is:
>>
>> @requires (vm.gc == null |   // null -> default -> g1 (usually)
>>            vm.gc == G1 ) &
>>            vm.gc.G1
>
> No, vm.gc.G1 is an alias to: (vm.gc == null | vm.gc == g1) &
> vm.gc.supportsG1.

Wow - on the one hand that is a compact/succinct syntax. On the other 
hand - no one will recognize what it actually means! I think I have 
missed a discussed I would like to have given input on!

>>
>> Aside: how does jtreg determine which GC's are supported?
> Sorry for not providing the full history here:
>
> CODETOOLS-7901583: jtreg should provide extensible mechanism for
> @requires properties
> - change to jtreg that allows a Test Suite to introduce its own
> @requires props
>
> JDK-8152432: Implement setting jtreg @requires properties vm.flavor,
> vm.bits, vm.compMode
> - implementation that mechanism in hotspot
>
> JDK-8154096: Extend WhiteBox API with methods which retrieve from VM
> information about available GC
> - fix which allows to know if a GC is supported
>
> JDK-8151283: Implement setting jtreg @requires property vm.isG1Supported.
> - introduction vm.gc.G1, vm.gc.Serial, vm.gc.Parallel and
> vm.gc.ConcMarkSweep
>
> JDK-8160088 (this one): update hotspot tests depending on GC to use
> @requires vm.gc.X
> - culmination: update tests to use new functionality

Wow I have missed out on a lot it seem! :(

What version of jtreg supports this level of customized @requires? 
Someone has already encountered the following error:

Error. Parse Exception: Syntax error in @requires expression: invalid 
name: XXX

for a new @requires XXX clause.

Thanks,
David

> Thanks,
> Dima
>
>>
>> Thanks,
>> David
>>
>>> Testing:
>>> 1) starting jtreg with various collectors with "-c"  option to verify
>>> correctness of test descriptions.
>>>     Number of selected tests before and after change is the same:
>>> -XX:+UseG1GC:  1,456
>>> -XX:+UseSerialGC: 1,366
>>> -XX:+UseParallelGC: 1,369
>>> -XX:+UseConcMarkSweepGC: 1,368
>>> Default:  1,483; error
>>>
>>> 2) RBT (in progress)
>>>
>>> 3) Diff is analyzed manually (only necessary lines are affected):
>>> #> hg diff |grep "^- " |sort -u
>>> - * @requires (vm.gc == "G1" | vm.gc == "null")
>>> - * @requires (vm.gc=="G1" | vm.gc=="null")
>>> - * @requires vm.gc == "G1" | vm.gc == "null"
>>> - * @requires vm.gc=="ConcMarkSweep" | vm.gc == "null"
>>> - * @requires vm.gc=="ConcMarkSweep" | vm.gc=="null"
>>> - * @requires vm.gc=="G1" | vm.gc =="null"
>>> - * @requires vm.gc=="G1" | vm.gc=="null"
>>> - * @requires vm.gc=="Parallel" | vm.gc=="null"
>>> - * @requires vm.gc=="Serial" | vm.gc=="null"
>>> - * @requires vm.gc=="null" | vm.gc=="G1"
>>> - * Copyright (c) 2013, 2014, Oracle and/or its affiliates. All rights
>>> reserved.
>>> - * Copyright (c) 2013, 2015, Oracle and/or its affiliates. All rights
>>> reserved.
>>> - * Copyright (c) 2014, 2015, Oracle and/or its affiliates. All rights
>>> reserved.
>>> - * Copyright (c) 2015, Oracle and/or its affiliates. All rights
>>> reserved.
>>> #> hg diff |grep "^+ " |sort -u
>>> + * @requires vm.gc.ConcMarkSweep
>>> + * @requires vm.gc.G1
>>> + * @requires vm.gc.Parallel
>>> + * @requires vm.gc.Serial
>>> + * Copyright (c) 2013, 2016, Oracle and/or its affiliates. All rights
>>> reserved.
>>> + * Copyright (c) 2014, 2016, Oracle and/or its affiliates. All rights
>>> reserved.
>>> + * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights
>>> reserved.
>>>
>>> Thanks,
>>> Dima
>