RFR (S): JDK-8129549 G1: Make sure the concurrnet thread does not mix its logging with the STW pauses

[Thomas Schatzl]

Hi,

On Tue, 2015-06-23 at 11:16 +0200, Bengt Rutisson wrote:
> Hi Per,
> 
> Thanks for looking at this!
> 
> On 2015-06-23 11:17, Per Liden wrote:
> > On 2015-06-23 10:09, Bengt Rutisson wrote:
> >>
> >>
> >> Hi everyone,
> >>
> >> Could I have a couple of reviews for this patch?
> >>
> >> http://cr.openjdk.java.net/~brutisso/8129549/webrev.00/
> >> https://bugs.openjdk.java.net/browse/JDK-8129549
> >
> > A few minor suggestions:
> >
> > concurrentMark.cpp
> > ------------------
> > 2996   if (_has_aborted || !cmThread()->during_cycle()) {
> >
> > To me it reads better if we first check if it's running before 
> > checking if it's aborted, i.e.:
> >
> > if (!cmThread()->during_cycle() || _has_aborted) ...
> >
> >
> > concurrentMarkThread.cpp
> > ------------------------
> >
> >   84 void ConcurrentMarkThread::cm_log(bool doit, const char* fmt, ...) {
> >   85   if (doit) {
> >   86     SuspendibleThreadSetJoiner sts_joiner;
> >
> > Could we make all logging in the CM thread use this method? This would 
> > mean we need another bool argument to tell the sts_joiner to be active 
> > or not.
> 
> Good points.
> 
> Here's an updated webrev:
> http://cr.openjdk.java.net/~brutisso/8129549/webrev.01/
> 
> Here's a diff compared to the first version:
> http://cr.openjdk.java.net/~brutisso/8129549/webrev.00-01.diff/


  looks good.

Thomas