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

[sangheon.kim]

Hi Thomas,

On 11/20/2017 10:17 AM, Thomas Schatzl wrote:
> 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)
webrev.2 looks good to me.

And thank you for addressing my comments on webrev.1.
I agree we can improve lazily allocating threads with JDK-8191082: Unify 
handling of thread sets when allocating them lazily.

Thanks,
Sangheon


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