RFR: 8040935: -XX:+AggressiveOpts broken: GC triggered before VM initialization completed on several tests

[Stefan Johansson]

Hi again,

On 2015-01-15 15:12, Stefan Johansson wrote:
> On 2015-01-15 00:49, Stefan Johansson wrote:
>> Hi Kim,
>>
>> On 2015-01-14 19:13, Kim Barrett wrote:
>>> On Jan 12, 2015, at 5:01 AM, Stefan Johansson 
>>> <stefan.johansson at oracle.com> wrote:
>>>> Please review this testfix to avoid the following issue:
>>>> https://bugs.openjdk.java.net/browse/JDK-8040935
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~sjohanss/8040935/hotspot.00
>>>>
>>>> Summary:
>>>> Making the test a driver test to avoid dealing with externally 
>>>> specified flags. One problematic flag is AggressiveOpts, it causes 
>>>> the VM to allocate more stuff during startup and that together with 
>>>> UseNUMA the eden space gets to little to handle all startup 
>>>> allocations. This will in turn lead to a GC being triggered before 
>>>> initialization is finished, and because of that failing the test.
>>>>
>>>> Testing:
>>>> * Tested via JPRT
>>> test/gc/TestNUMAPageSize.java
>>> Summary: Changed test to not be affected by external options.
>>>
>>> I'm not sure I like this approach to resolving the bug.  I think it
>>> results in many potentially interesting configurations never being
>>> tested, e.g. this tests with one and only one GC, the default GC.  It
>>> ignores the nightly build cycling through various configurations to
>>> increase test coverage.
>>>
>> Thanks for taking a look at this. I see your point and think we 
>> should do something better. The test was originally written to test a 
>> specific regression we had when starting with -Xmx8m and -XX:+UseNUMA 
>> with the default collector, but that doesn't mean we shouldn't test 
>> it for the others as well. I'm not sure if we have any NUMA support 
>> for the other collectors today, but no harm in testing that the 
>> option doesn't cause any problems.
>>
>> I see two ways of improving the coverage, one would be to use 
>> @requires to say that AggressiveOpts can't be true, but that will 
>> still leave the possibility to set other options that might push us 
>> over the limit of allocations during startup. The other way would be 
>> to extend the test to run with all collectors explicitly, but ignore 
>> any options passed in. This would of course still have less coverage 
>> than today, but at least all collectors would be covered.
>>
>> I would prefer the second one, it would be robust and the coverage 
>> would be ok.
>>
>
> Updated webrev that tests all GCs:
> http://cr.openjdk.java.net/~sjohanss/8040935/hotspot.01/
>
> Verified that the test still passes when run through JPRT.
>
I think I've changed my mind. After some discussions here in Stockholm, 
and after seeing that gc/TestSmallHeap.java fails for the same reason, 
I'm starting to think that @requires is the way to go. We don't want to 
re-write all tests using small heaps just because it doesn't work with 
AggressiveOpts, that will loose too much test coverage and I think we 
should solve similar issues the same way.

New proposal:
http://cr.openjdk.java.net/~sjohanss/8040935/hotspot.02

Stefan

> Thanks,
> Stefan
>
>> Thanks,
>> Stefan
>>
>