RFR: 8160088: update hotspot tests depending on GC to use @requires vm.gc.X
David Holmes
david.holmes at oracle.com
Fri Jun 24 09:20:30 UTC 2016
Hi Dima,
This is diverging somewhat but ... :)
On 24/06/2016 6:54 PM, Dmitry Fazunenko wrote:
> Hi David,
>
> On 24.06.2016 9:48, David Holmes wrote:
>> Thanks for the info Dima! I may have missed it but I think this new
>> @requires functionality needs some broader advertising. :)
>
> That functionality was introduced on this alias as well:
> http://mail.openjdk.java.net/pipermail/hotspot-dev/2016-April/022443.html
> But I agree, "Implement setting jtreg @requires properties vm.flavor,
> vm.bits, vm.compMode" doesn't
> attract much attention.
> If I were from sales, I would write: "Braking news: jtreg introduces
> smart keywords" :)
>
>>
>> I'm still not 100% sure what something like vm.gc.G1 means though.
>> Looking at this comment:
>>
>> /**
>> * For all existing GC sets vm.gc.X property.
>> * Example vm.gc.G1=true means:
>> * VM supports G1
>> * User either set G1 explicitely (-XX:+UseG1GC) or did not set any GC
>>
>> I think it would have been more accurate to add "and the selected GC
>> is G1" - right?
>
> No. vm.gc.G1 means:
> The test needs to set G1 GC via -XX:+UseG1GC
> It requires:
> 1) vm supports G1 (minimal VM doesn't)
> 2) User didn't say: -XX:+UseSerialGC ,-XX:+UseParallelGC -XX:+UseCMSGC
> (otherwise it will be a flag conflict)
> Note: User may say: -XX:+UseG1GC
>
>> That is what the logic seems to do (though I'm not sure why it checks
>> both current and currentSetByErgo ??)
>
> There is a class of machines where ergonomics selects Serial GC if no
> other GC is specified explicitly.
> For such machines currentGC might be "Serial" but setting G1 is still
> possible.
> currentSetByErgo == false means: no other collector but currently
> selected is available for setting via -XX:UseXGC.
I'm more confused than I thought I was then. :(
Let's pretend that default GC selection is random. If vm.gc == null we
are letting the VM choose the GC by whatever means. If the test requires
G1 then we need to know that currentGC() == G1 - if it is something else
then we can't run this test. It doesn't seem to matter how the currentGC
was selected as long as it is G1 - so I don't see why we need to query
if the currentGC was selected by ergo ??
Cheers,
David
> I know that the current description is hard to understand, but I didn't
> come up with a better wording.
>
>>
>> Anyway for this set of test changes everything seems fine.
>
> Thanks a lot!
>
> -- Dima
>
>>
>> Thanks,
>> David
>>
>> On 23/06/2016 10:52 PM, Dmitry Fazunenko wrote:
>>> David,
>>>
>>> On 23.06.2016 14:00, David Holmes wrote:
>>>> 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!
>>>
>>> It was discussed on hotspot-gc-dev alias as part of '8151283' review:
>>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2016-June/018352.html
>>>
>>>
>>> Originally it was proposed names like: vm.gc.G1.acceptable
>>> but after input from GC dev, vm.gc.G1 was selected.
>>> Documentation how the properties are set what that mean is available in
>>> the class source (jtreg plugin):
>>> <jdk-root>/test/jtreg-ext/requires/VMProps.java
>>>
>>>
>>>>
>>>>>>
>>>>>> 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.
>>>
>>> It was implemented in the final build of jtreg4.1 and of course it's
>>> available in jtreg4.2
>>> Quiting jtreg/doc/jtreg/tag-spec.html:
>>>
>>> |requires.extraPropDefns <source-files>|
>>>
>>> This option is used to provide source files for classes that will be
>>> used at the beginning of each test suite run, to determine
>>> additional characteristics of the system for use with the
>>> |@requires| tag. Each class must implement
>>> |java.util.concurrent.Callable<java.util.Map<String, String>>|. When
>>> invoked, the |call()| method should return properties that can be
>>> referenced in an expression in a |@requires| tag. /Note:/ the names
>>> of the new properties that may be returned by calling these classes
>>> must also be declared in a |requires.properties| entry.
>>>
>>> If this option is specified, the following additional options may
>>> also be specified:
>>>
>>> * |requires.extraPropDefns.libs <source-files>| — source files for
>>> classes that will be put on the classpath when the primary
>>> classes are run.
>>> * |requires.extraPropDefns.bootlibs <source-files>| — source files
>>> for classes that will be put on the bootclasspath when the
>>> primary classes are run.
>>> * |requires.extraPropDefns.javacOpts <options>| — options that
>>> will be passed to javac when the source files are compiled.
>>> * |requires.extraPropDefns.vmOpts <options>| — options that will
>>> be passed to VM when the classes are executed.
>>>
>>> In this family of options, if a source file is enclosed in square
>>> brackets, no error message will be given if the file is not
>>> available.
>>>
>>> The following properties may appear in either TEST.ROOT or any
>>> TEST.properties file:
>>>
>>> The reason, why @requires XXX is not recognized by jtreg - XXX is not
>>> registered in TEST.ROOT.
>>> See: hotspot/test/TEST.ROOT for example on how to register new props.
>>>
>>> For the time being recently introduced requires properties are available
>>> only in the hotspot repository.
>>> To introduce them in jdk - a few lines from hotspot/test/TEST.ROOT
>>> should be placed in jdk/test/TEST.ROOT
>>> We haven't done this yet, because we are considering a possibility to
>>> move VM specific tests from jdk into hotspot repo.
>>>
>>> Thanks,
>>> Dima
>>>
>>>
>>>>
>>>> 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
>>>>>
>>>
>
More information about the hotspot-dev
mailing list