RFR (XS): 8200730: Fix debug=gc+phases time tracking in Remark and Cleanup

[Thomas Schatzl]

Hi Stefan,

On Mon, 2018-04-16 at 14:09 +0200, Stefan Johansson wrote:
> Hi Thomas,
> 
> On 2018-04-16 13:14, Thomas Schatzl wrote:
> > Hi all,
> > 
> >    ping for any reviewer...
> > 
> > Thanks,
> >    Thomas
> > 
> > On Wed, 2018-04-04 at 14:26 +0200, Thomas Schatzl wrote:
> > > Hi all,
> > > 
> > >    can I have reviews to let the recently introduced timing
> > > measurements in Remark and Cleanup pauses actually show useful
> > > numbers?
> > > 
> > > The problem is that the GCTraceTime instances were not assigned
> > > to a variable inside a scope so the compiler immediately
> > > destructed it, always giving "0.000ms" lengths.
> > > 
> > > Also added to use the gc timer to these lines for JFR support.
> > > 
> > > I would like to think this is a trivial change.
> > > 
> > > CR:
> > > https://bugs.openjdk.java.net/browse/JDK-8200730
> > > Webrev:
> > > http://cr.openjdk.java.net/~tschatzl/8200730/webrev
> 
> Sorry for missing this one :)
> 
> Thanks for fixing this. Two things:
> src/hotspot/share/gc/g1/g1ConcurrentMark.cpp
> 1994     GCTraceTime(Debug, gc)("Clear Next Bitmap");
> 
> This line should to be changed as well.

That's already been wrong for a long time -__-

> ---
> 
> The second thing is a partly pre-existing problem and I suggest you
> fix all occurrences in this file. The name "trace" is a bit
> misleading when the debug-level is "Debug", which is the case for all
> GCTraceTime in this file. I would prefer using "debug", like on this
> line:
> 1654     GCTraceTime(Debug, gc, phases) debug("Weak Processing", 
> _gc_timer_cm);
> -----

Fixed too.

http://cr.openjdk.java.net/~tschatzl/8200730/webrev.0_to_1/ (diff)
http://cr.openjdk.java.net/~tschatzl/8200730/webrev.1/ (full)

Thanks for your review.

Thomas