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

Thomas Schatzl thomas.schatzl at oracle.com
Tue Jul 16 08:31:45 UTC 2019


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"?

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.

Thanks for your review,
  Thomas




More information about the hotspot-gc-dev mailing list