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

[Dmitry Fazunenko]

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