<p>Hi Bengt,</p>
<p>Do you think it's worthwhile to add asserts to the double[] setters in G1GCPhaseTimes to ensure that worker_i is within bounds? It may not segfault but stomp memory instead if it's outside the bounds.  Maybe overkill though ...</p>

<p>Also, should/can worker_i be uint? Or are negative values expected? Or would this require casts from the caller because the rest of the relevant code uses int?</p>
<p>Regards,</p>
<p>VitalyVitaly</p>
<p>Sent from my phone</p>
<div class="gmail_quote">On Jun 20, 2012 9:14 AM, "Bengt Rutisson" <<a href="mailto:bengt.rutisson@oracle.com">bengt.rutisson@oracle.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Hi everyone,<br>
<br>
Could I please have some reviews for this change:<br>
<a href="http://cr.openjdk.java.net/~brutisso/7178361/webrev.00/" target="_blank">http://cr.openjdk.java.net/~<u></u>brutisso/7178361/webrev.00/</a><br>
<br>
Background<br>
As part of the PrintGC and PrintGCDetails logging there is information about how long the GC pause was. The timing of the pause was done differently in G1 depending on whether PrintGC or PrintGCDetails were enabled. It turns out that PrintGCDetails was just timing part of the pause.<br>

<br>
This change will make both PrintGC and PrintGCDetails use the same timing. To achieve this I had to refactor the code a bit. I moved all the timing data out of G1CollectorPolicy into a new class called G1GCPhaseTimes.<br>

<br>
My intention is that this change should not alter the format of the output of PrintGC or PrintGCDetails. It should just correct the timing data.<br>
<br>
However, I did find that we are collecting timing information about card counts, under an #ifdef. I moved this to the finest log level instead. This does not change the existing format for normal usage of PrintGC or PrintGCDetails.<br>

<br>
Also, I had to update how the TraceGen0Time data is logged. I will have another look at this, but my idea was to leave the format exactly as it was. However, I think the format has decayed over time so maybe I'll try to clean it up.<br>

<br>
I intend to follow this change up with a change to remove the special treatment of the single threaded case for PrintGCDetails (tracked in CR 7178363).<br>
<br>
Finally, this work revealed an issue with how the ergonomics in G1 measure the collection pauses. I did not want to change this behavior now so I filed a separate RFE for that (7178365: G1: Ergonomics only count part of the collection pause).<br>

<br>
Bengt<br>
</blockquote></div>