RFR: 8138920: Refactor the sampling thread from ConcurrentG1RefineThread

[Thomas Schatzl]

Hi all,

On Fri, 2015-10-09 at 15:49 -0400, Derek White wrote:
> Hi Bengt,
> On 10/9/15 12:01 PM, Bengt Rutisson wrote:

> > Hi Derek,
> > Comments inlined.
> > On 2015-10-09 00:29, Derek White wrote:
> > > Another call for review:
> > > 2nd webrev:
> > > http://cr.openjdk.java.net/~drwhite/8138920/webrev.02/
> > > See changes and comments below:
> > > On 10/8/15 2:47 AM, Bengt Rutisson wrote:

> > > > On 2015-10-07 17:19, Derek White wrote:
> > > > > On 10/7/15 4:02 AM, Bengt Rutisson wrote:

> > > > > > On 2015-10-06 19:51, Derek White wrote:
> > > > > > 
[...]

> > > > > > You write "and also fixes a P4 bug". Which bug is that?
> > > > > JDK-8136856 G1 makes two concurrent refinement threads with
> > > > > -XX:G1ConcRefinementThreads=1
> > > > > (because the sampling thread "is-a" concurrent refinement
> > > > > thread.
> > > > 
> > > > Ah. I see. Makes sense. Thanks.
> > > > 
> > > > But it is still not possible to turn refinement off by setting
> > > > -XX:G1ConcRefinementThreads=0 since that is considered the
> > > > default, right?
> > > 
> > > I'm not sure about this. If I recall correctly, Thomas implied
> > > that it was hard to disable concurrent refinement without
> > > disabling the sampling thread too.

It is actually impossible to disable the concurrent refinement threads
without also disabling the sampling thread at the moment, since
ConcurrentG1Refine manages it.
I.e. G1CollectedHeap uses it, but ConcurrentG1Refine manages it.

Sampling thread ownership should also move to G1CollectedHeap (or
G1CollectorPolicy as it mainly used to feed information to it).

There is a somewhat similar problem about disabling concurrent
refinement threads with the hot card cache, in particular the card
counts table. G1CollectedHeap manages it, ConcurrentG1Refine uses it...

> > I'm thinking about this code in Arguments::set_g1_gc_flags():
> > 
> >   if (G1ConcRefinementThreads == 0) {
> >     FLAG_SET_DEFAULT(G1ConcRefinementThreads, ParallelGCThreads);
> >   }
> > 
> > Could we change that, so that you could turn off refinement by
> > setting -XX:G1ConcRefinementThreads=0 ? It should probably be
> > handled as a separate fix though.
> > 
> It sounded like Thomas has plans in this area already.

No particular plan. The problem here is backwards compatibility, so the
only way I could see is add another flag (*hides quickly*).

In any case this would be a separate fix.

Thanks,
  Thomas