RFR: 8138920: Refactor the sampling thread from ConcurrentG1RefineThread

[Derek White]

Hi Per,

On 10/26/15 10:56 AM, Per Liden wrote:
> On 2015-10-26 15:47, Derek White wrote:
>> Hi Per,
>>
>> On 10/26/15 3:09 AM, Per Liden wrote:
>>> Hi Derek,
>>>
>>> On 2015-10-23 19:43, Derek White wrote:
>>>> RFR 5.
>>>>
>>>> This responds to Per's last comments and Thomas's suggestions for 
>>>> better
>>>> methods names and comments.
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~drwhite/8138920/webrev.05/
>>>> Webrev 05 vs 04:
>>>> http://cr.openjdk.java.net/~drwhite/8138920/webrev.05.vs.04/
>>>>
>>>> RFE: JDK-8138920 <https://bugs.openjdk.java.net/browse/JDK-8138920>
>>>> Refactor the sampling thread from ConcurrentG1RefineThread
>>>
>>> The patch looks good, but it looks like you forgot to address the
>>> following two comments.
>>
>> Ahh, I got the Monitor name, but forgot the thread name. Thanks Per!
>
> Derek, note that my first comment below about _monitor is not about 
> its name, but about how it's instantiated, i.e. please remove the 
> dynamic allocation. Fixing the name is also good, but that was not 
> what my comment was about.

OK. Sorry for rushing past this.

  - Derek
>
> cheers,
> /Per
>
>>
>>   - Derek
>>>
>>> [...]
>>>>>>> g1YoungListSampleThread.cpp
>>>>>>> ---------------------------
>>>>>>>
>>>>>>>   59   _monitor = new Monitor(Mutex::nonleaf,
>>>>>>>   60                          "Sample thread monitor",
>>>>>>>   61                          true,
>>>>>>>   62 Monitor::_safepoint_check_never);
>>>>>>>
>>>>>>> It looks like _monitor could be a value member instead of a 
>>>>>>> pointer,
>>>>>>> to avoid the extra allocation here.
>>>>>>>
>>>>>>>   66   set_name("G1 Sample");
>>>>>>>
>>>>>>> This should probably be updated to reflect the new name.
>>>
>>> I don't need to see a new webrev, but please address these before
>>> pushing.
>>>
>>> cheers,
>>> /Per
>>