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

[Mikael Gerdin]

Thomas,

On 2015-03-18 13:39, Thomas Schatzl wrote:
> 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.

The problem is that verification (VerifyBefore/AfterGC) also uses 
G1RootProcessor, but does not want the phase times reported (at least 
not in the global phase times instance).
So in the end we need some way to determine if we should report the time 
or not for each root step.

/Mikael

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