RFR(s): 8151283: Implement setting jtreg @requires property vm.isG1Supported.

Dmitry Fazunenko dmitry.fazunenko at oracle.com
Mon Jun 20 21:29:21 UTC 2016


Igor,

Thanks a lot for the review!

On 20.06.2016 23:35, Igor Ignatyev wrote:
> Dima,
>
> the fix looks good to me, thanks for implementing that. I have a question about -Xbootclasspath/a:bootClasses, is it required? I thought classes from requires.extraPropDefns.bootlibs will be added to BCP by jtreg.

Yes, it's required. We might decided for example to use 
-Xbootclasspath/*p*: instead.

bootClasses - is the name of the local folder (under scratch dir) where
classes from the'requires.extraPropDefns.bootlibs' will be compiled to. And I haven't 
mentioned, that I introduced a field, which is currently unused: private 
static final WhiteBox WB = WhiteBox.getWhiteBox(); This is for the 
simplification of use in the future. Thanks, Dima

>
> Thanks,
> — Igor
>> On Jun 20, 2016, at 6:12 PM, Dmitry Fazunenko <dmitry.fazunenko at oracle.com> wrote:
>>
>> Hello,
>>
>> During the review period of this CR a couple of changes adding new requires properties were integrated:
>>   8157831: vm.simpleArch
>>   8158412: vm.flightRecorder
>>
>> Merged variant:
>> http://cr.openjdk.java.net/~dfazunen/8151283/webrev.02/
>>
>> Thanks,
>> Dima
>>
>>
>> On 16.06.2016 16:34, Dmitry Fazunenko wrote:
>>> Hi Michail,
>>>
>>> This is TEST.ROOT file, which defines itself what is the root for libraries:
>>>    48 # Path to libraries in the topmost test directory. This is needed so @library
>>>    49 # does not need ../../ notation to reach them
>>>    50 external.lib.roots = ../../
>>>
>>> So, for the stuff like VMProps we need to use "../.." in paths
>>>
>>> Thanks,
>>> Dima
>>>
>>> On 16.06.2016 16:20, Michail Chernov wrote:
>>>> Hi Dima,
>>>>
>>>> I have a question about paths:
>>>>
>>>>   requires.extraPropDefns = ../../test/jtreg-ext/requires/VMProps.java
>>>> -requires.properties=sun.arch.data.model
>>>> +requires.extraPropDefns.bootlibs = ../../test/lib/sun
>>>>
>>>>
>>>> Do we need to use relative paths instead of full? We use full path in tests:
>>>>
>>>> TestSmallHeap.java: * @library /testlibrary /test/lib /test/lib/share/classes
>>>>
>>>> Thanks,
>>>> Michail
>>>>
>>>> On 06/14/2016 07:14 PM, Dmitry Fazunenko wrote:
>>>>> Hi Stefan,
>>>>>
>>>>> The idea of 'vm.gc.X.acceptable' was an ability to introduce other GC related properties like 'supported', 'default', 'setByErgonomics', 'deprecated', etc.
>>>>> But setting 'vm.gc.X' won't prevent us from adding  'vm.gc.X.Y' later when needed.
>>>>>
>>>>> I fully agree:   @requires vm.gc.G1 looks much nicer and causes less questions.
>>>>>
>>>>> New edition:
>>>>> http://cr.openjdk.java.net/~dfazunen/8151283/webrev.01/
>>>>>
>>>>> Thanks for suggestion that,
>>>>> Dima
>>>>>
>>>>>
>>>>> On 13.06.2016 10:46, Stefan Johansson wrote:
>>>>>> Hi Dima,
>>>>>>
>>>>>> I don't know the code and the way this is invoked good enough to do a proper review, but I have a style comment. Wouldn't it be nice to only call the value "vm.gc.G1", and have:
>>>>>> @requires vm.gc.G1
>>>>>>
>>>>>> I feel that "acceptable" provide very little information and what we want to @require is that the test is only run with G1 (when supported) and I think that is clear from such syntax. Comments?
>>>>>>
>>>>>> Thanks,
>>>>>> Stefan
>>>>>>
>>>>>> On 2016-06-10 18:04, Dmitry Fazunenko wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> I'm looking for a couple of Reviewers for a simple change, which allows to skip tests depending on non supported garbage collectors.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~dfazunen/8151283/webrev.00/
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8151283
>>>>>>>
>>>>>>> The fix introduces four new requires boolean properties:
>>>>>>>       vm.gc.G1.acceptable
>>>>>>>       vm.gc.Serial.acceptable
>>>>>>>       vm.gc.Parallel.acceptable
>>>>>>>       vm.gc.ConcMarkSweep.acceptable
>>>>>>> Which are set by the jtreg plugin prior the test execution starts.
>>>>>>> After that fix we will be able to replace in tests:
>>>>>>>     @requires vm.gc == null | vm.gc == "G1"
>>>>>>> with
>>>>>>>     @requires vm.gc.G1.acceptable
>>>>>>>
>>>>>>> 'vm.gc.G1.acceptable' will be evaluated to 'false' not only if other GC is set externally, but also if G1 is not supported.
>>>>>>> In other words the fix allows not to run GC specific tests in not applicable configurations.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Dima
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/attachments/20160621/1670d9a5/attachment.htm>


More information about the hotspot-gc-dev mailing list