Trivial bug in psScavenge.cpp?

[Thomas Schatzl]

Hi,

On Mon, 2013-10-28 at 09:10 +0100, Bengt Rutisson wrote:
> Hi Jeremy,
> 
> On 2013-10-26 04:35, Jeremy Manson wrote:
> > Hi folks,
> >
> > Following my habit of reporting trivial bugs (and perhaps my habit of being
> > wrong about their being bugs).  I've been playing around with the
> > event-based JVM tracing API, since we had our own version of the same thing
> > internally (amusingly, we called our timer TraceGCTime instead of
> > GCTraceTime, but it did almost exactly the same thing).  I noticed this in
> > psScavenge.cpp:
> >
> >        469     GCTraceTime tm("StringTable", false, false, &_gc_timer);
> >        470     // Unlink any dead interned Strings and process the remaining
> > live ones.
> >        471     PSScavengeRootsClosure root_closure(promotion_manager);
> >        472     StringTable::unlink_or_oops_do(&_is_alive_closure,
> > &root_closure);
> >
> > That's all well and good, but surely you want curly braces before line 469
> > and after line 472?  Otherwise it credits a lot more time to string table
> > processing than is appropriate...
> 
> Nice catch!
> 
> You are absolutely right. This section used to be inside an if 
> statement, but that was removed and suddenly the timing got the wrong scope.
> 
> Would you like to file a bug and provide a patch, or would you like me 
> to help out with any of it?

I filed the CR already at
https://bugs.openjdk.java.net/browse/JDK-8027364 - still, maybe you want
to provide a webrev/patch :)

Thomas