RFR (M): 8074037: Refactor the G1GCPhaseTime logging to make it easier to add new phases

Thomas Schatzl thomas.schatzl at oracle.com
Tue Mar 10 10:39:14 UTC 2015


Hi Bengt,

On Tue, 2015-03-10 at 10:48 +0100, Bengt Rutisson wrote:
> Hi Erik,
[...]
> To get this email thread back on track - I'm still looking for reviews 
> for the latest webrev:
> http://cr.openjdk.java.net/~brutisso/8074037/webrev.03/
> 

- g1CollectedHeap.cpp:3754, active_workers should be a uint I think to
match the parameter types.

- minor style issue (feel free to ignore): the change stores the
G1GCPhaseTimes instances into local variables a few times. That variable
is sometimes called "timer", "phase_times". Also e.g. G1CLDClosure may
benefit from introducing a local variable.

- another (feel free to ignore): in the long sum starting with
g1CollectorPolicy.cpp:1134 the operators are sometimes at the end of the
preceding line, sometimes before. I would prefer them at the end of the
preceding line.

- thanks for removing the PRAGMA's in g1GCPhaseTimes.cpp. :)

Looks good otherwise.

Thanks,
  Thomas






More information about the hotspot-gc-dev mailing list