Request for review (M): 7172279: G1: Clean up TraceGen0Time and TraceGen1Time data gathering
bengt.rutisson at oracle.com
Tue May 29 06:58:18 UTC 2012
I should also have mentioned that this change should not alter the log
output. (Except for the ParallelGCThreads = 0 case which now will look
the same as PrallelGCThreads >= 1 case.)
I tested this by leaving the old code in and comparing the output from
the old code with the new code. In my tests they look exactly the same.
On 2012-05-28 22:37, Bengt Rutisson wrote:
> Hi all,
> Can I have a couple of code reviews for this change?
> I am preparing a change to remove the special treatment of the single
> threaded case for G1 PrintGCDetails output. We should be able to use
> the same code for both ParallelGCThreads = 0 and PrallelGCThreads >=
> 1. This will hopefully simplify the logging code.
> In preparation for that change I would like to clean up the
> TraceGen0Time and TraceGen1Time data gathering code a bit. It is
> currently kind of difficult to follow the code and there is no clear
> distinction between the data gathered for these two cases compared to
> the data gathered for PrintGCDetails output.
> Here is what I tried to do:
> - Remove the unnecessary define_num_seq macro and the multiple
> inheritance of the Summary class.
> - Replaced the above with two classes, TraceGen0TimeData and
> TraceGen1TimeData, that will encapsulate the data needed.
> - Made sure that the data is only updated if the corresponding flags
> are turned on.
> - Removed the separate "non-parallel" case. All data is available even
> with ParallelGCThreads = 0.
> - Removed the ExitAfterGCNum flag. I hardly think this is an
> appropriate product flag to have in the code.
> - Removed some unused methods.
> - Removed 7 year old assert(true) with accompanying comments.
> - Removed the unused aux time concept.
> There was one thing that I was hesitant to remove, but finally decided
> to remove anyway. It was the G1CollectorPolicy::check_other_times()
> method. First I added the same method to TraceGen0TimeData. But it is
> a lot of code and I am not sure it is very useful. It can't do any
> exact checks and it seems unlikely to me that it will detect any
> issues. Also, it seems to me like the code was in the wrong place. I
> would have preferred it in some kind of verify method rather than in a
> print method.
More information about the hotspot-gc-dev