RFR: 8066122: CollectionUsageThreshold.java times out when run with -XX:+ExplicitGCInvokesConcurrent

[Dmitry Fazunenko]

Looks good to me. But you need to get a reviewer's approval.

Thanks,
Dima

On 25.12.2014 15:20, Michail Chernov wrote:
> Hi Dmitry,
>
> Tests were changed, now they use @requires.
>
> http://cr.openjdk.java.net/~eistepan/~mchernov/8066122/webrev.03/
>
> Thanks,
> Michail
>
> On 25.12.2014 14:53, Dmitry Fazunenko wrote:
>> Hi Michail,
>>
>> I strongly believe, that the right way to fix this issue is adding 
>> @requires as you proposed originally.
>> If one specifies external VM options it is expected that all tested 
>> VM will be run with those options.
>> If a test can't be run with some options, adding @requires will help 
>> to not run.
>> Trying to alter the external flags within tests is like a hack we had 
>> to do to avoid failures. Before @requires there wasn't another 
>> alternative.
>> But now, we have got a mechanism to express that a test doesn't work 
>> with certain options. Let's use it.
>>
>> Thanks,
>> Dima
>>
>>
>> On 24.12.2014 17:42, Michail Chernov wrote:
>>> Hi,
>>>
>>> Please take a look at this fix.
>>>
>>> Webrev: 
>>> http://cr.openjdk.java.net/~eistepan/~mchernov/8066122/webrev.02/
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8066122
>>>
>>> Thanks,
>>> Michail
>>>
>>>
>>> On 22.12.2014 17:21, Michail Chernov wrote:
>>>> Hi Kim,
>>>>
>>>> About jtreg and options filtering out - I know that Boolean options 
>>>> filtering is not bug, it is feature. For example, in case if we 
>>>> define -XX:+SomeImportanatOptionForThisTest for testing and this 
>>>> options is undefined by VM flags which we define during testing, it 
>>>> causes to wrong test result.
>>>>
>>>> I looked through other tests and found that I was wrong - 
>>>> improvement for test failure is more simpler than I thought. 
>>>> Currently I don't make any change in testlibrary.
>>>>
>>>> Suggestid fix is to add -XX:-ExplicitGCInvokesConcurrentto all 
>>>> RunUtil.runTestClearGcOpts and RunUtil.runTestKeepGcOpts invocation.
>>>>
>>>> http://cr.openjdk.java.net/~eistepan/~mchernov/8066122/webrev.02/
>>>>
>>>> Thanks,
>>>> Michail
>>>>
>>>> On 17.12.2014 20:33, Kim Barrett wrote:
>>>>> [This is still not a real review, just kibitzing so far - I’ve 
>>>>> looked at the changes, but don’t yet feel I understand the 
>>>>> surrounding code well enough to say I’ve reviewed them.]
>>>>>
>>>>> On Dec 17, 2014, at 11:01 AM, Michail Chernov 
>>>>> <michail.chernov at oracle.com> wrote:
>>>>>> Test was fixed, now it excludes -XX:+ExplicitGCInvokesConcurrent 
>>>>>> option from test run.
>>>>>> Also fixed LowMemoryTest.java - it failed with same reason as 
>>>>>> CollectionUsageThreshold.java.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~eistepan/~mchernov/8066122/webrev.01/
>>>>>>
>>>>>> Function getFilteredTestJavaOpts in Util.java was got from 
>>>>>> hotspot repo http://hg.openjdk.java.net/jdk9/hs-gc/hotspot/ from 
>>>>>> testlibrary, because Util.java in GC and Util.java in Runtime 
>>>>>> repos are different. This function can be useful not only for 
>>>>>> this case.
>>>>>>
>>>>>> Simply adding of -XX:-ExplicitGCInvokesConcurrent won't work 
>>>>>> because JTREG filtersout it in case if 
>>>>>> -XX:+ExplicitGCInvokesConcurrent is defined as VM option.
>>>>> That bit about jtreg filtering out 
>>>>> -XX:-ExplicitGCInvokesConcurrent seems backward, at least for a 
>>>>> case like this.  I wouldn’t have expected global options 
>>>>> applicable to all tests to override test-specific arguments. Also, 
>>>>> it would seem that would make the other tests my earlier comment 
>>>>> was referring to not work either.  So I’m confused about why 
>>>>> there’s all this extra filtering going on in this change and why 
>>>>> it would be needed.  Unless there are important differences in 
>>>>> some of the underlying infrastructure, like the different 
>>>>> testlibrary variants (jdk/test/lib/testlibrary vs hotspot’s 
>>>>> testlibrary?).  Sorry I’m not being clearer about this - I’m 
>>>>> suspicious there is a deeper problem, but don’t know enough to 
>>>>> point to its whereabouts.  (And that’s assuming my suspicions are 
>>>>> correct, and I’m not just confused.)
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>
>>
>>
>