Request for review (S): 8006398: Add regression tests for deprectated GCs

Bengt Rutisson bengt.rutisson at oracle.com
Fri Feb 15 11:51:54 UTC 2013


On 2/14/13 9:06 PM, Jon Masamitsu wrote:
>
> On 2/14/2013 6:32 AM, Bengt Rutisson wrote:
>>
>> Hi Jon,
>>
>> Are you ok with me pushing this? We can always add more tests later.
>
> Your call.  I myself would do it now so I don't have to think about it
> again but that's because I so easily forget about things :-)

OK. My memory is probably not that good either. :) I added the test 
TestCMSNoIncrementalMode.java:

http://cr.openjdk.java.net/~brutisso/8006398/webrev.02/

I also changed the folder name from gc/start-warnings to 
gc/start_warnings since a "-" in the name makes it difficult to filter 
these tests out in JPRT. The -rtests option thinks that "-" is a 
separator for the platform definitions.

Thanks,
Bengt

>
> Ship it.
>
> Jon
>>
>> Thanks,
>> Bengt
>>
>> On 2/13/13 9:11 AM, Bengt Rutisson wrote:
>>>
>>> Hi John,
>>>
>>> Thanks for looking at this!
>>>
>>> On 2/13/13 3:40 AM, Jon Masamitsu wrote:
>>>> Bengt,
>>>>
>>>> Changes look good.
>>>>
>>>> You include tests for UseG1GC and UseParallelGC (should
>>>> not print warnings) but not UseConcMarkSweepGC nor
>>>> UseSerialGC.  Why the former but not the latter?
>>>
>>> The TestSerialGC.java is running with UseSerialGC. Am I missing 
>>> something or is this the test you meant?
>>>
>>> For CMS I think you have a point. I was kind of thinking that the 
>>> test TestParNewCMS was anyway testing that GC combination, but we 
>>> should really have a separate CMS test as well. I just added 
>>> TestCMS.java to test UseConcMarkSweepGC explicitly. Here is an 
>>> updated webrev:
>>>
>>> http://cr.openjdk.java.net/~brutisso/8006398/webrev.01/
>>>
>>>>
>>>> Do you want to test
>>>>
>>>> -XX:+UseConcMarkSweepGC -XX:-CMSIncrementalMode -version
>>>>
>>>> to check that no warning is emitted?
>>>
>>> Hm. I don't know. If we start combining all the flags we will get 
>>> into an explosion of tests. I think this is a valid combination to 
>>> test. But in that case so are things like "-XX:+UseConcMarkSweepGC 
>>> -XX:-UseSerialGC". I like the way you think about this testing, I 
>>> just think I would like to keep these tests to be smoke testing the 
>>> more commonly used combinations.
>>>
>>> Make sense?
>>>
>>> Thanks again for looking at this!
>>> Bengt
>>>
>>>>
>>>> Jon
>>>>
>>>> On 2/12/2013 4:56 AM, Bengt Rutisson wrote:
>>>>>
>>>>> Hi again,
>>>>>
>>>>> Christian has pushed his test library change and it has propagated 
>>>>> to the hotspot-gc repository. So, this change is ready to be 
>>>>> pushed. Just need some reviews. Any takers? :)
>>>>>
>>>>> Thanks,
>>>>> Bengt
>>>>>
>>>>> On 1/16/13 1:58 PM, Bengt Rutisson wrote:
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> Could I have a couple of reviews for this change?
>>>>>> http://cr.openjdk.java.net/~brutisso/8006398/webrev.00/
>>>>>>
>>>>>> Recently we deprecated some GC combinations. Those should now 
>>>>>> print a warning at startup. Other GC combinations should not 
>>>>>> print any warnings.
>>>>>>
>>>>>> With the new process handling support that Christian Törnqvist is 
>>>>>> adding to the JTREG tests for hotspot it is very easy to write 
>>>>>> test that start a VM and checks the output.
>>>>>>
>>>>>> This changes makes use of Christian's testlibrary to verify that 
>>>>>> warnings are printed as expected.
>>>>>>
>>>>>> I'm also adding the "gc" keyword to JTREG to make it possible to 
>>>>>> filter out GC tests. We should probably use this for all test in 
>>>>>> the the /gc folder, but I think that should be done as a separate 
>>>>>> change.
>>>>>>
>>>>>> The webrev above is based on Christian's webrev to add the 
>>>>>> testlibrary:
>>>>>> http://cr.openjdk.java.net/~brutisso/8006413/webrev.00/
>>>>>>
>>>>>> Thanks,
>>>>>> Bengt
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>




More information about the hotspot-gc-dev mailing list