RFR (M) : 8024319 : Add regression tests on documented GC-related -XX:+Print* options

[Filipp Zhinkin]

Mikael,

thanks for looking at it!

On 10/07/2013 06:25 PM, Mikael Gerdin wrote:
> Filipp,
>
> On 10/03/2013 12:50 PM, Filipp Zhinkin wrote:
>> Hi,
>>
>> may I get one more review for this change?
>
> Sorry for taking so long to get to this.
>
> Please don't add @author-tags to the tests, hg stores that information 
> fine.
Yes, sure.
>
> TestPrintGCTools seems like it should be somewhere in the /testlibrary 
> folder since it's not a test and just a utility class for testing gc 
> log output.
I agree with you.
There is a one another RFR on tests for heap ratio options, I'll made 
applicable changes there as well.
> test/gc/arguments/print/TestCMSPrintGCFlags.java:
> DefNew + CMS is deprecated in JDK 8. Do we still want a test for that?
I can't see nothing bad in it, because on the one hand it does not 
require any specific test cases
and on the other hand it verifies that it will work for someone who 
decided to use DefNew + CMS despite the deprecation.
>
> test/gc/arguments/print/TestG1PrintGCFlags.java:
> I think it would be useful to have an example of a matching record for 
> this test.
Yep, it does not looks obvious. I'll add a comment.
>
> test/gc/arguments/print/TestPrintGCAppTimeFlags.java:
> Just pass the flags as strings to the function instead of passing 
> booleans.
>  48         //-XX:-PrintGCApplicationConcurrentTime 
> -XX:-PrintGCApplicationStoppedTime
>   49         testGCAppTime(false, false, options);
>   50         //-XX:-PrintGCApplicationConcurrentTime 
> -XX:+PrintGCApplicationStoppedTime
>   51         testGCAppTime(false, true,  options);
It is there to simplify results parsing: "options" may already contain 
PrintGCApplication*Time options,
so to make a decision whether or not appropriate strings are expected in 
test output I'll have to check
not just occurance of -XX:[+-]PrintGCApplication*Time, but find all 
PrintGCApplication*Time entries and
check the sign before the last one.
There is nothing hard in such check, but passing booleans looks much 
simpler.
>
> test/gc/arguments/print/TestPrintGCTaskTimeStampsFlag.java:
> Add some sample VM output in a comment.
> "Simple sanity check for PrintGCTaskTimeStamps entries." simple? A 150 
> line parser class isn't exactly a simple sanity check :)
Well, maybe it was a mistake to name it simple. :) I'll add an example.
>
> test/gc/arguments/print/TestPrintTenuringDistributionFlag.java:
> Why do you add +UseAdaptiveSizePolicy? It's only supported (and 
> enabled by default) in +UseParalell{Old}GC
This test designed to work with any options passed to it, so 
+UseAdaptiveSizePolicy was added for the case when
options contains -XX:-UseAdaptiveSizePolicy to make sure that Parallel 
GC will calculate new survivor size after GC
and print tenuring distribution.
>
> test/gc/arguments/print/TestSerialAndParallelPrintGCFlags.java:
> ParNew + Serial is deprecated in JDK 8. Do we still want a test for that?
The same logic as with test/gc/arguments/print/TestCMSPrintGCFlags.java 
applies here too.

Thanks,
Filipp.
>
> /Mikael
>
>
>>
>> Thanks,
>> Filipp.
>>
>> On 09/18/2013 01:16 AM, Jesper Wilhelmsson wrote:
>>> Looks good!
>>> I ran the tests and they passed. I introduced a bug in the verbose
>>> logging and the relevant test failed. Awesome!
>>>
>>> /Jesper
>>>
>>> Filipp Zhinkin skrev 17/9/13 6:14 PM:
>>>> I'll be very appreciated if someone will take a look at it.
>>>>
>>>> Thanks,
>>>> Filipp.
>>>>
>>>> On 09/10/2013 01:15 PM, Filipp Zhinkin wrote:
>>>>> Hi,
>>>>>
>>>>> I'd like to get a review on new regression tests on various 
>>>>> GC-related
>>>>> -XX:+Print* options:
>>>>>
>>>>> http://bugs.sun.com/view_bug.do?bug_id=8024319
>>>>>
>>>>> webrev:
>>>>> http://cr.openjdk.java.net/~kshefov/8024319/webrev.00/
>>>>>
>>>>> Brief overview of tests to be added:
>>>>> Following tests verify that PrintGC and PrintGCDetails flags work
>>>>> and emit
>>>>> appropriate output.
>>>>> Tests grouped by GC in order to handle different GC logging format.
>>>>> gc/arguments/print/TestCMSPrintGCFlags
>>>>> gc/arguments/print/TestG1PrintGCFlags.java
>>>>> gc/arguments/print/TestSerialAndParallelPrintGCFlags
>>>>>
>>>>> Test on PrintGCTimeStamps and PrintGCDateStamps flags. It verifies 
>>>>> that
>>>>> options and their
>>>>> combinations emit appropriate output and verifies that time and date
>>>>> stamps
>>>>> are not decreasing over time.
>>>>> gc/arguments/print/TestGCStampFlags
>>>>>
>>>>> Test on PrintGCApplicationConcurrentTime and
>>>>> PrintGCApplicationStoppedTime.
>>>>> It verifies that these options emit appropriate output.
>>>>> gc/arguments/print/TestPrintGCAppTimeFlags
>>>>>
>>>>> Test on PrintGCTaskTimeStamps flah. It verifies that when this
>>>>> option emit
>>>>> output only with parallel GC
>>>>> and also verifies that all timestamps are not decreasing and for
>>>>> each GC
>>>>> thread verifies that number of
>>>>> reported task timestamps equal to expected number of tasks.
>>>>> gc/arguments/print/TestPrintGCTaskTimeStampsFlag
>>>>>
>>>>> Test on PrintTenuringDistribution flag. It verifies that appropriate
>>>>> output is
>>>>> emitted when this option is
>>>>> turned on and also verifies that reported values lies between 0 and
>>>>> selected
>>>>> MaxTenuringThreshold.
>>>>> gc/arguments/print/TestPrintTenuringDistributionFlag*
>>>>>
>>>>> *Testing done: manual testing and JPRT.
>>>>>
>>>>> Thanks,
>>>>> Filipp.
>>>>>
>>>>>
>>>>
>>
>