Request for review (M): 7178363 G1: Remove the serial code for PrintGCDetails and make it a special case of the parallel code

[Bengt Rutisson]

Hi all,

Here is an updated webrev for this change:
http://cr.openjdk.java.net/~brutisso/7178363/webrev.01/

It turns out the the earlier change for 7178361 had introduced two more 
issues: the heap transition information for the PrintGC output and the 
output for evacuation failures had both been moved in an unintended way. 
The above webrev corrects both of these chagne too. Thanks John 
Cuthbertson for pointing me to the evacuation failure output.

Bengt


On 2012-07-19 11:01, Bengt Rutisson wrote:
>
> Hi again,
>
> Just in case anybody already started looking at this: I have updated 
> the webrev since I had to make some changes to make it compile with 
> the NMT fixes that have just made it into the hotspot-gc repository. 
> Those updates were just to make it compile and not really related to 
> my change, so I just overwrote the existing webrev. Just use the same 
> link as before:
>
> http://cr.openjdk.java.net/~brutisso/7178363/webrev.00/
>
> Also, if you want to see what the new output looks like I am attaching 
> a file called new.txt with an example from running SpecJBB2005 with 
> this command line:
>
> -XX:+UseG1GC -XX:ParallelGCThreads=4 -XX:+UnlockExperimentalVMOptions 
> -XX:G1LogLevel=finest -XX:+TraceGen0Time -Xms256m -Xmx2G
>
> I am also attaching a file called old.txt with what the output, using 
> the same command line, looked like before my change. As you can see 
> the differences are what I listed in my earlier email. You will also 
> notice that the "old" version has an entry for the SATB filtering, 
> even though all the entries are 0 (we didn't do a concurrent cycle so 
> there has been no SATB filtering). This was a bug I just introduced 
> with my last change (for 7178361), so the new example is more correct.
>
> Thanks,
> Bengt
>
> On 2012-07-18 15:55, Bengt Rutisson wrote:
>>
>> Hi everyone,
>>
>> I would like some reviews of this change:
>> http://cr.openjdk.java.net/~brutisso/7178363/webrev.00/
>>
>> This removes the special treatment for ParallelGCThreads=0 from the 
>> G1 logging. I did keep the log output unchanged for that case. 
>> Basically it just has one indentation level less and skips some 
>> output. I am not sure this is really necessary since it is really a 
>> special case. I'm open to change that special treatment too and just 
>> have the same output as for ParallelGCThreads=1.
>>
>> The PrintGCDetails log output should be the same as before with three 
>> minor adjustments:
>>
>> - The "Sum" is now not printed for the start and end values for GC 
>> workers. This sum does not really make sense to me.
>>
>> - The "(ms)" unit was removed from output that aren't in milliseconds 
>> (termination attempts for example).
>>
>> - The average value is now printed as a double for all types.
>>
>> I tried to clean up the code a bit and introduced a separate class, 
>> Snippet WorkerDataArray, to keep track of the per thread logging. I 
>> also introduced getters and setters to avoid having to make 
>> G1CollectorPolicy and TraceGen0TimeData friend classes to 
>> G1GCPhaseTimes.
>>
>> Thanks,
>> Bengt
>