Request for review (M): 7172279: G1: Clean up TraceGen0Time and TraceGen1Time data gathering
Bengt Rutisson
bengt.rutisson at oracle.com
Mon May 28 20:37:07 UTC 2012
Hi all,
Can I have a couple of code reviews for this change?
http://cr.openjdk.java.net/~brutisso/7172279/webrev.01/
Background
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.
Thanks,
Bengt
More information about the hotspot-gc-dev
mailing list