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

[Michail Chernov]

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.)
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>
>
>