RFR: JDK-8150068: Log the main G1 phases at info level

[Thomas Schatzl]

Hi Bengt,

On Mon, 2016-02-22 at 13:19 +0100, Bengt Rutisson wrote:
> Hi again,
> 
> Now that JDK-8149013 has been pushed the only remaining caller of 
> G1GCPhaseTimes::accounted_time_ms() was G1GCPhaseTimes::print(). It 
> makes the code easier to follow if the calculation of the accounted
> time 
> is inlined in the print method. Here's an updated webrev where 
> accounted_time_ms() has been removed and inlined in the print()
> method 
> instead.
> 
> http://cr.openjdk.java.net/~brutisso/8150068/webrev.01/
> 

  looks okay (let me have another look later or tomorrow) except for
the changes related measuring the time of the HCC. It looks complicated
now. Instead of simply recording lots of 0.0 values in the scanHCC
WorkerDataArray, the code trying to read from it needs to know that the
HCC is enabled everywhere.

Maybe add some flag in WorkerDataArray if you really dislike that (I do
not see a particular problem with it), that indicates that that phase
has been skipped (or the inverse) instead of checking
 ConcurrentG1Refine::hot_card_cache_enabled() a few times.

This also adds needless dependencies when trying to get that time. So I
would prefer either reverting that part of the changes, or hiding the
fact that the phase has not been executed (has no measurements) within
the G1GCPhaseTimes.

I mean, G1GCPhaseTimes::average_time_ms() could return 0.0 by itself in
this case.

Also, in G1CollectorPolicy::record_collection_pause_end, in this
particular case I would somewhat prefer to repeat the call to
average_time_ms instead of using a local variable. The method is so
big, adding another reference to a local defined 100 lines above its
use seems only to make it harder to read. I have no strong opinion
about this though, but maybe Mikael has.

Thanks,
  Thomas