RFR (M): 8027962: Per-phase timing measurements for strong roots processing

Thomas Schatzl thomas.schatzl at oracle.com
Wed Mar 18 12:39:05 UTC 2015

Hi Bengt,

  thanks for taking this over...

On Wed, 2015-03-18 at 12:45 +0100, Bengt Rutisson wrote:
> Hi everyone,
> When this was first sent out for review I suggested a couple of cleanup 
> changes before doing this change. One of the cleanups has been pushed:
> 8074037: Refactor the G1GCPhaseTime logging to make it easier to add new 
> phases
> https://bugs.openjdk.java.net/browse/JDK-8074037
> The other one is out for review and is close to being pushed:
> 8075210: Refactor strong root processing in order to allow G1 to evolve 
> separately from GenCollectedHeap
> https://bugs.openjdk.java.net/browse/JDK-8075210
> Based on the above patches, here's an updated webrev to fix 8027962:
> http://cr.openjdk.java.net/~brutisso/8027962/webrev.02
> Could I have some reviews for this change?

- G1GCPhaseTimes::note_gc_start does not use the mark_in_progress
parameter any more and can be removed.

- this is more some style issue: since we moved all G1 specific root
processing to G1RootProcessor, we actually do not need to pass around
the phase_times parameter any more.
We already assume that the G1GCPhaseTimes instance is global in the
entire G1 code, so code in G1RootProcessor could also just access it via
the global.
Just store it in a local variable to save typing at the beginning of the
method. Actually G1RootProcessor::evacuate_roots() does exactly that
unlike process_vm_roots() and process_java_roots().
It is up to you to decide.

- I am fine with the move of "SATB filtering" to log level finest and
the additional indentation.


More information about the hotspot-gc-dev mailing list