RFR (M): 8190426: Lazily initialize refinement threads with UseDynamicNumberOfGCThreads

[Thomas Schatzl]

Hi all,

  Stefan Johansson found another issue: in the original code, before
activating a thread it checked whether that thread had already been
activated. The changes did not do that.

Here's a new webrev:

http://cr.openjdk.java.net/~tschatzl/8190426/webrev.1_to_2/ (diff)
http://cr.openjdk.java.net/~tschatzl/8190426/webrev.2/ (full)

Testing:
hs tier1+2

Thanks,
  Thomas


On Mon, 2017-11-20 at 12:26 +0100, Thomas Schatzl wrote:
> Hi Sangheon,
> 
>   thanks for your review, and sorry for the late answer - I have been
> travelling lately...
> 
> On Thu, 2017-11-09 at 15:00 -0800, sangheon.kim wrote:
> > Hi Thomas,
> > 
> > On 11/03/2017 09:28 AM, Thomas Schatzl wrote:
> > > Hi,
> > > 
> > >    can I have reviews for this change that makes refinement
> > > threads
> > > behave the same as the other thread groups we have in G1?
> > > 
> > > I.e. with UseDynamicNumberOfGCThreads enabled (which is off by
> > > default) they are created lazily.
> > > 
> > > This helps a little bit further with startup/footprint benchmarks
> > > (if
> > > enabled).
> > > 
> > > CR:
> > > https://bugs.openjdk.java.net/browse/JDK-8190426
> > > Webrev:
> > > http://cr.openjdk.java.net/~tschatzl/8190426/webrev/
> > > Testing:
> > > hs-tier1+2
> > 
> > I like this idea, thank you for improving this.
> > 
> > I have some comments.
> 
> I think I addressed all your concerns. Some of them were due to me
> being confused about which memory allocations cause VM exit, and
> which
> don't. I hope I got that right now.
> 
> I also changed the behavior of the thread startup to be more similar
> to
> the worker threads, namely:
> - always start one refinement thread at startup
> - support InjectGCWorkerCreationFailure (which is tested by our tests
> btw)
> - make error messages and failure modes more similar to the ones for
> the work gangs.
> 
> Even with that we want to look at JDK-8191082 for better specifying
> the
> expected behavior.
> 
> Webrevs:
> http://cr.openjdk.java.net/~tschatzl/8190426/webrev.0_to_1/ (diff)
> http://cr.openjdk.java.net/~tschatzl/8190426/webrev.1/ (full)
> Testing:
> hs-tier1+2
> 
> Thanks,
>   Thomas
>