RFR: 8065749: [TESTBUG]: gc/arguments/TestG1HeapRegionSize.java fails at nightly

[Bengt Rutisson]

Hi Dima,

On 2014-11-24 15:03, Dmitry Fazunenko wrote:
> Hi Bengt,
>
>
> On 24.11.2014 18:33, Bengt Rutisson wrote:
>>
>> Hi Evgeniya,
>>
>> On 2014-11-24 14:13, Evgeniya Stepanova wrote:
>>> Hi,
>>>
>>> Could you please review changes for 8065749 ?
>>> Test TestG1HeapRegionSize.java is G1-specific and could not work 
>>> without -XX:+UseG1GC passed directly.
>>> Option added to the run.
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8065749
>>> fix: http://cr.openjdk.java.net/~eistepan/8065749/webrev.00/
>>
>> I think your suggested fix will work, but I have a question regarding 
>> the original change.
>>
>> Before the original change the test explicitly checked that it was 
>> run with G1 and just skipped otherwise:
>>
>> http://hg.openjdk.java.net/jdk9/hs-gc/hotspot/comparison/6464714dd742/test/gc/arguments/TestG1HeapRegionSize.java
>>
>> Was this test really failing at all? I thought we were only going to 
>> add @requires to tests that were failing due to conflicting GC 
>> combinations. If it was working before I think a different approach 
>> to fix the current problem would be to revert back to the old code 
>> without @requires.
>
> You are right, it was our mistake. This test wasn't reverted when we 
> decided to update only failing tests.
> Yes, a possible solution would be just to roll back the previous 
> change, but I don't think it makes sense just to roll back.
>
> What I suggest:
> 1) keep this test G1 specific (@requires g1 and +UseG1 explicitly) - 
> what is proposed by this change
> 2) make tests GC independent - remove @requires and check in tests 
> that RegionSize == <expected> in case of G1, and RegionSize == 0 in 
> case of another GC.
>
> I don't think that original variant (silently pass if not g1) is the 
> right solution.

I don't think 2) makes sense. G1HeapRegionSize is not set to 0 for other 
collectors and there is really no reason to do that either. So, testing 
the value of that with other GCs is just a waste of time.

Considering that, I don't really see why 1) is better than reverting to 
the original code. After all, we are not sure how we want @requires to 
work and in my mind it is better to use it as little as possible until 
we have figured out how it should work.

Thanks,
Bengt

>
> What do you think?
>
> -- Dima
>
>
>
>
>
>>
>> Thanks,
>> Bengt
>>
>>>
>>> Thanks,
>>> Evgeniya Stepanova
>>>
>>> -- 
>>> /Evgeniya Stepanova/
>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20141125/53988bb4/attachment.htm>