RFR (S): 8076463: Add logging for the preserve CM referents task

[Jon Masamitsu]

On 2/25/2016 8:18 AM, Thomas Schatzl wrote:
> Hi Jon,
>
> On Thu, 2016-02-25 at 07:56 -0800, Jon Masamitsu wrote:
>> Thomas,
>>
>> Why did you decide to move preserve_cm_referents() out of
>> process_discovered_references()?  The work done in
>> preserve_cm_referents() has to be performed before it
>> is safe to do the work in process_discovered_references()
>> so it seems safer to call preserver_cm_referents() at
>> the beginning of preserve_discovered_references().
>    - to preserve the kind-of pattern in the methods that do some
> parallel work.
>
> I am kind of trying to all make them look like:
>
> void G1CollectedHeap::some_phase(G1ParScanThreadStateSet*
> per_thread_state) {
>    cur_time = get_current_time_stamp();
>
>    do_work();
>
>    log_duration(cur_time);
> }
>
> to at some point change the first and last line into a scoped object,
> or probably better move the scoped object out of the method call.
>
> So that the code (maybe) looks like:
>
> {
>    ScopedObjectForTiming x();
>    do_x(per_thread_states);
> }
>
> {
>    ScopedObjectForTiming y();
>    do_y(per_thread_states);
> }
>
> [...]
>
> Also, the code then also corresponds more to the logging, having every
> log line at "top level" without any nesting.
>
> - preserve cm referents is kind of reference processing part 0 to me
> (if process_discovered_references is part 1 as described in the comment
> in line g1CollectorPolicy.cpp:4500). Similar to reference enqueuing it
> seems (and needs) to be a completely separate phase of what
> process_discovered_references() does.
>
> - looking at existing code, we already split out the reference
> enqueueing into an extra method to be called in the
> post_evacuate_collection_set() method (I know that there is a
> dependency with the Clear CT phase right now, but I am kind of mulling
> over completely removing it after some... other changes).
>
> If you think it is still better to call preserve_cm_referents() within
> process_discovered_references(), I will change it.

You can leave the change as is.

Reviewed.

Jon

>
> Thanks,
>    Thomas