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