RFR (S): 8227084: Add timing information for merge heap root preparation

[sangheon.kim at oracle.com]

Hi Thomas,

On 7/16/19 1:31 AM, Thomas Schatzl wrote:
> Hi Sangheon,
>
> On Mon, 2019-07-15 at 14:12 -0700, sangheon.kim at oracle.com wrote:
>> Hi Thomas,
>>
>> On 7/10/19 8:36 AM, Thomas Schatzl wrote:
>>> Hi all,
>>>
>>>    can I have reviews for this change that adds timing information
>>> for
>>> parts of the Merge Heap Roots phase as preparation for JDK-8226913.
>>>
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8227084
>>> Webrev:
>>> http://cr.openjdk.java.net/~tschatzl/8227084
>>   Looks good.
>>
>> Just minor knit:
>>
>>   419   debug_time("Prepare Merge Heap Roots",
>> _cur_optional_prepare_merge_heap_roots_time_ms);
>>   434   debug_time("Prepare Merge Heap Roots",
>> _cur_prepare_merge_heap_roots_time_ms);
>> - Line 419 seems to include 'optional'? I don't need extra webrev for
>> this.
>>
>    this is correct as far as I can see. Line 419 adds the timing for the
> "optional" output, while 434 for the regular output.
>
> They do have the same name though, so maybe that is irritating you? So
> you suggest that 419 should be "Prepare Optional Merge Heap Roots"?
Yes, this is what I wanted to spot.

>
> I regenerated the webrev in place, see
> http://cr.openjdk.java.net/~tschatzl/8227084/webrev/src/hotspot/share/gc/g1/g1GCPhaseTimes.cpp.frames.html
>   line 419.
Looks good, thanks for fixing this too.

Sangheon


>
> Thanks for your review,
>    Thomas
>