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

[Thomas Schatzl]

Hi,

On Thu, 2019-07-11 at 17:30 -0400, Kim Barrett wrote:
> > On Jul 10, 2019, at 11:36 AM, Thomas Schatzl <
> > thomas.schatzl at oracle.com> 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
> > Testing:
> > hs-tier1-5
> > 
> > Thanks,
> >  Thomas
> 
> Looks good.
> 
> Just one minor thing that you can change or not, as you prefer.
> 
> -------------------------------------------------------------------
> -----------
> src/hotspot/share/gc/g1/g1RemSet.cpp
> 1129       assert(merge_remset_phase == G1GCPhaseTimes::MergeRS,
> "Wrong merge phase");
> 1137       assert(merge_remset_phase == G1GCPhaseTimes::MergeRS,
> "Wrong merge phase");
> 
> I'm not sure these assertions carry their weight anymore.  They used
> to verify consistency between two arguments, but now
> merge_remset_phase is computed locally from the argument whose value
> is used in the immediately enclosing if-statement.

I regenerated the webrev with those removed.

> 
> An aside: We really need a helper somewhere for expressions like
> 
>    total.seconds() * 1000.0
> 
> I know that Tickspan::milliseconds() doesn't do what we want, because
> it returns an integral value.
> 

I know, it's getting annoying for me as well. It also causes
unnecessary bugs.

I will see if there is anything that can be extracted from the
unfinished changes of JDK-8201313 I have.

Thanks,
  Thomas