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

[Igor Ignatyev]

Dima,

thank you for explanation. is ‘bootClasses’ documented somewhere? if no, could you please write a comment in TEST.ROOT?

Thanks,
— Igor
> On Jun 21, 2016, at 12:29 AM, Dmitry Fazunenko <dmitry.fazunenko at oracle.com> wrote:
> 
> 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 
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>> 
>