RFR (XS) JDK-8160055 Misplaced call to ClassLoaderDataGraph::clear_claimed_marks during initial mark

[Thomas Schatzl]

Hi Joe,

On Wed, 2016-09-28 at 12:39 -0400, Joseph Provino wrote:
> Please review the latest webrev: 
> http://cr.openjdk.java.net/~jprovino/8160055/webrev.01
> 
> There is a WhiteBox test now.
> 
> thanks.
> 
> joe
> 

- the log message itself should be consistent with other messages, i.e.
no "Time" at the end. (This is all timing output anyway).

- I would prefer to have it listed within "Other" times, at a lower log
level. Maybe only print it if its value is > 0.0 too at this time.
However I think the categorization will be fixed anyway soon, but at
least this phase does not seem to warrant a top level category. (yes,
"Merge PSS" is wrong too, but as mentioned, it will be fixed).

- since the message is printed every time, but updated only during
initial mark, this will cause confusing output after the first initial
mark. I.e. you will need to initialize its value in
G1GCPhaseTimes::note_gc_start().

- the member of g1GCPhaseTimes should be consistent with other
messages, i.e. have a "_recorded" prefix and a "_ms" suffix. Same with
the method name and method parameters to record the time.

- is there a reason there is a standalone test for this and not adding
checking of this message to test/gc/g1/TestGCLogMessages.java as
suggested in the previous reply at the bottom?

- if there is a reason:
  - there is no need to run the test in "othervm" mode.
  - the test should have a @requires vm.gc.G1 tag.

Thanks,
  Thomas


> 
> On 8/29/2016 7:47 AM, Thomas Schatzl wrote:
> > 
> > Hi,
> > 
> > On Tue, 2016-08-16 at 14:15 -0400, Joseph Provino wrote:
> > > 
> > > Please review this very small change.
> > > CR: JDK-8160055 Misplaced call to
> > > ClassLoaderDataGraph::clear_claimed_marks during initial mark
> > > Webrev: http://cr.openjdk.java.net/~jprovino/8160055/webrev.00
> > > Thanks.
> > > joe
> > > 
> >    - the recorded time also needs to be printed, in
> > G1GCPhaseTimes::print(), somewhere in the "Other" category. Please
> > only print it if that phase has been executed.
> > 
> >    - please try to expand tests/gc/g1/TestGCLogMessages.java to
> > trigger an initial mark (triggering a marking round via whitebox
> > should be sufficient) and check whether this log message appears.
> > 
> > Thanks,
> >    Thomas
> >