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

[Bengt Rutisson]

Hi Dima,

On 2015-01-22 12:42, Dmitry Fazunenko wrote:
> Hi Bengt!
>
> Thanks for looking at the tests and sharing your thoughts!
>
> Yes, it was considered to use ProcessTool but we decided to introduce 
> AbstractPringGCTest class.
> Our thoughts:
> - log generation and log analysis are two different tasks, and they 
> should logically separated:

Ok, but that's how it is with ProcessTool as well. Only it is much 
clearer how the output you analyze is connected to the process you ran 
if you use ProcessTool. I don't like how using AbstractPrintGCTest 
requires that you name the -Xloggc file correct.

>     * starting GCTask with various options  (one line @run 
> main/othervm ... GCTask)

GCTask is fine. The name is a bit odd, but I understand the need for a 
common class to do allocations to trigger GC.

>     * log analysis (the java code within sources)

Right. And you use the OutputAnalyzer just like you would if you were 
using ProcessTool.

> this increases readability

I disagree that adding a new abstract class increases readability.

> - it's possible to provide several checkers for the same log
>     @run main/othervm ... GCTask
>     @run main TestCheck1
>     @run main TestCheck2

You can grab the output from the ProcessTool and pass it to several 
checkers too. I don't see the differences.

> ...
>     @run main TestCheck3
>   or provide the same check for several logs (not currently implemented)
>     @run main/othervm ... -loggc:log1 GCTask
>     @run main/othervm ... -loggc:log2 GCTask
>     @run main/othervm ... -loggc:log3 GCTask
>     ...
>     @run main TestCheck log1 log2 log3

Again, I don't see how AbstractPringGCTest is any different here 
compared to just grabbing the output from the ProcessTool.

>
> - writing log to a dedicated file will guarantee, that program output 
> and the GC log will not be mixed up.
>   (not valid argument for that particular case, bun in general that's 
> true)

That's a valid point. But it shouldn't be much of a problem since the 
tests are under our control. It is up to us what we log on System.out.

>
> - using @run main/othervm will allow to *not ignore* external options 
> (jtreg -vmoptions:...), that makes such tests applicable for wider 
> range of configurations and check, that for example -Xcomp doesn't 
> turn off PrintGCDetails...

You can pass the external options on to ProcessTool.

>
>
> Regarding AbstractPringGCTes: it doesn't duplicate any functionality, 
> it just reads content of a text file.

Yes, but getting the output from a process is what ProcessTool does. So, 
it duplicates the functionality of getting the GC output.

>
> Regarding ProcessTools. Yes, it's possible to develop tests with this 
> library. This library itself is very good and powerful. But I 
> personally would not recommend using it for test development because 
> it duplicates harness functionality. Unfortunately, jtreg currently 
> doesn't provide all functionality required for VM testing and we have 
> to use ProcessTools as workaround.
> And people already got used to ProcessTools and like this style. But 
> in long term, there will be the same problem with support of such 
> tests, as we experience now with tests written in shell. Indeed, using 
> ProcessTools is like using java for shell scripting.

The long term support will be made more complex if we introduce yet 
another way of doing the same thing.

>
> Returning to Denis' tests. They intentionally do not use ProcessTools. 
> They could be used as example demonstrating an alternative approach.

Yes, I think it would be interesting to see what the tests would look 
like based on ProcessTool.

Thanks,
Bengt

>
> Thanks,
> Dima
>
>
>
> On 22.01.2015 10:26, Bengt Rutisson wrote:
>>
>> On 2015-01-22 08:20, Bengt Rutisson wrote:
>>>
>>> Hi Denis,
>>>
>>> Not a full review, but I have a question.
>>>
>>> It seems like the AbstractPrintGCTest is kind of duplicating what 
>>> ProcessTools already does. Have you considered using 
>>> ProcessTools.createJavaProcessBuilder(..) instead of the @run 
>>> commands to automatically get the process control and log support 
>>> instead of introducing the AbstractPrintGCTest class?
>>
>> Here's an example of how I mean that you can use 
>> ProcessTools.createJavaProcessBuilder() instead:
>>
>> http://hg.openjdk.java.net/jdk9/hs-gc/hotspot/file/e6a0cfbfdc9a/test/gc/g1/TestGCLogMessages.java 
>>
>>
>> Bengt
>>
>>>
>>> Thanks,
>>> Bengt
>>>
>>> On 2015-01-20 16:49, denis kononenko wrote:
>>>> Hi All,
>>>>
>>>> Could you please review new tests on GC-related -XX:Print options.
>>>>
>>>> Webrev link: 
>>>> http://cr.openjdk.java.net/~eistepan/dkononen/8024319/webrev.00/
>>>> Bug id: https://bugs.openjdk.java.net/browse/JDK-8024319
>>>> Testing: automated
>>>> Description:
>>>>
>>>> There is a group of new tests to test the following GC options:
>>>>
>>>> -XX:+-PrintAdaptiveSizePolicy
>>>> -XX:+-PrintGCApplicationConcurrentTime
>>>> -XX:+-PrintGCApplicationStoppedTime
>>>> -XX:+-PrintGCDateStamps
>>>> -XX:+-PrintGCDetails
>>>> -XX:+-PrintGC
>>>> -XX:+-PrintGCTimeStamps
>>>> -XX:+-PrintTenuringDistribution
>>>>
>>>> Each of the tested options has a pair of corresponding tests, one 
>>>> is for testing an enabled option and another for disabled. The 
>>>> tests are simple parsers of GC's logger output and looking for a 
>>>> specific marker corresponding to the tested option. The output is 
>>>> provided by another process which is implemented in GCTask.java. 
>>>> It's necessary because we have to guarantee that GC's log has been 
>>>> completely written and committed to the filesystem before we start 
>>>> analyzing it. The most obvious solution is to politely finish the 
>>>> writing process. Thus the every test spawns an auxiliary process 
>>>> which produces GC's log file, waits for its finish, loads the 
>>>> output and then performs actual testing. These steps are 
>>>> implemented with jtreg's annotations and a helper class which can 
>>>> be found AbstractPrintGCTest.java. This class encapsulates reading 
>>>> GC's log output from the log file and provides that output to the 
>>>> tests.
>>>>
>>>> To get GC's logger working GCTask forces the garbage collecting 
>>>> process. It attempts to consume all memory available to the young 
>>>> generation by creating a lot of unreferenced objects. Sooner or 
>>>> later the garbage collector shall be invoked. In favor of 
>>>> performance the task is implemented to be ran with a small memory 
>>>> size less or equal to 128 megabytes. This is excplicitly specified 
>>>> with -Xmx JVM's option in jtreg's annotations.
>>>>
>>>> Please note that some options work for specific GCs only. To 
>>>> prevent them from being executed against wrong GC jtreg's 
>>>> annotations and groups are used.
>>>>
>>>> Thank you,
>>>> Denis.
>>>>
>>>>
>>>>
>>>
>>
>