RFR (S): 8035398: Add card redirty time in "Other" time in G1

[Bengt Rutisson]

Hi Thomas,

On 2/25/14 2:52 PM, Thomas Schatzl wrote:
> Hi all,
>
>    can I have reviews for the following small change? It does nothing but
> adding a "Redirty Cards" message showing the time the card redirtying
> took as part of the "Other" time in the GC log.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8035398
>
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8035398/webrev/

This looks good. Thanks for adding this logging!

A couple of minor nits. Feel free to ignore them, I am fine with the 
patch as it is.

g1CollectedHeap.cpp

5291 
g1_policy()->phase_times()->record_redirty_logged_cards_time_ms((os::elapsedTime() 
- redirty_logged_cards_start) * 1000.0);

This is a very long line. I think I would prefer to store 
"os::elapsedTime() - redirty_logged_cards_start) * 1000.0" in a local 
variable to make it more readable.



g1GCPhaseTimes.cpp

  317   if (G1DeferredRSUpdate) {
  318     print_stats(2, "Redirty Cards", 
_recorded_redirty_logged_cards_time_ms);
  319   }

Most of the other "optional" values use a check like  if 
(_recorded_redirty_logged_cards_time_ms > 0.0) instead of checking 
flags. I am not sure what is better, I just thought it might be more 
consistent to use that here too.


TestGCLogMessages.java.
Should we add a test without G1DeferredRSUpdate enabled?

Thanks,
Bengt
>
> Testing:
> jprt
>
> Thanks,
>    Thomas
>