RFR (L): 8024319: Add regression tests on documented GC-related -XX:+Print* options
bengt.rutisson at oracle.com
Thu Jan 22 12:51:25 UTC 2015
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
> 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
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.
> 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:
>>> 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:
>>>> Bug id: https://bugs.openjdk.java.net/browse/JDK-8024319
>>>> Testing: automated
>>>> There is a group of new tests to test the following GC options:
>>>> 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
>>>> 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,
More information about the hotspot-gc-dev