RFR (XS): JDK-8047976: Ergonomics for GC thread counts should update the flags

[Sangheon Kim]

Hi Jesper,

Thank you for sponsoring this change.
I will send you once the fix is ready on other email.

Thanks,
Sangheon


On 09/26/2014 04:39 AM, Jesper Wilhelmsson wrote:
> Thomas Schatzl skrev 26/9/14 10:40:
>> Hi Sangheon,
>>
>> On Fri, 2014-09-26 at 10:04 +0200, Bengt Rutisson wrote:
>>> Hi all,
>>>
>>> Here's an updated webrev from Sangheon:
>>>
>>> http://cr.openjdk.java.net/~brutisso/8047976/webrev.02/
>>>
>>>   From Sangheon:
>>>
>>> "This includes Thomas's comments about:
>>> 1) Moving value set to Arguments::set_g1_gc_flags()
>>> 2) Adding jtreg test for G1ConcRefinementThreads variable"
>>
>> The changes in the Hotspot code look okay to me. Thanks!
>>
>> Some comments about the test case code:
>>
>> - There is a typo in AUTO_SELCT_THREADS_COUNT, should be
>> AUTO_SELECT_THREADS_COUNT I think.
>> - There is no need to have @build and @run lines if you do not use
>> special options to invoke the test afaik.
>
> Once this is fixed I'm fine with this change.
> If you send an "hg export" of the change I'll push it.
> /Jesper
>
>>
>> Otherwise looks okay.
>>
>> Thanks,
>>    Thomas
>>
>>
>>>
>>> Thanks,
>>> Bengt
>>>
>>>
>>> On 2014-09-25 11:01, Thomas Schatzl wrote:
>>>> Hi,
>>>>
>>>> On Thu, 2014-09-25 at 09:32 +0200, Thomas Schatzl wrote:
>>>>> Hi,
>>>>>
>>>>> On Wed, 2014-09-24 at 08:51 -0700, Sangheon Kim wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> can I have reviews for the following small change to update
>>>>>> G1ConcRefinementThreads variable?
>>>>>> And also I need a sponsor for this.
>>>>>>
>>>>>> CR:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8047976
>>>>>>
>>>>>> Webrev:
>>>>>> http://cr.openjdk.java.net/~brutisso/8047976/webrev.00/
>>>>>> <http://cr.openjdk.java.net/%7Ebrutisso/8047976/webrev.00/>
>>>>> Please update the flag during argument processing, not every time the
>>>>> method is called.
>>>>>
>>>>> Somewhere in Arguments::set_g1_gc_flags() would be appropriate I 
>>>>> think.
>>>>>
>>>> Please also add a jtreg test case. At least one test case that 
>>>> verifies
>>>> that with G1 the G1ConcRefinementThreads is not zero if nothing or 
>>>> zero
>>>> is passed, and another one that checks that the value passed is taken
>>>> over.
>>>>
>>>> The test could parse the output of -XX:+PrintFlagsFinal. The test
>>>> gc/arguments/TestObjectTenuringFlags.java shows how this is done.
>>>>
>>>> There is also some behavior that seems to be wrong: if you manually 
>>>> set
>>>> the number of G1ConcRefinementThreads to zero (which means
>>>> "automatically select"), -XX:+PrintFlagsFinal still prints
>>>> "G1ConcRefinementThreads := 0". Imo even in this case the output 
>>>> should
>>>> contain the actual number of threads used.
>>>>
>>>> At least if you set ConcGCThreads manually to 0, the flag is set to 
>>>> the
>>>> actual value after argument processing.
>>>>
>>>> ParallelGCThreads also shows 0 here, but I think this part of bug
>>>> JDK-6979279 and should be fixed there.
>>>>
>>>> Thanks,
>>>>     Thomas
>>>>
>>>>
>>>
>>
>>