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