RFR: 8138920: Refactor the sampling thread from ConcurrentG1RefineThread

[Per Liden]

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.

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
>