Trivial bug in psScavenge.cpp?

[Jeremy Manson]

It's pretty trivial, so unless my being involved will help anything, you
guys can feel free to take care of it without me.

Jeremy


On Mon, Oct 28, 2013 at 1:37 AM, Thomas Schatzl
<thomas.schatzl at oracle.com>wrote:

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