Request for code review - JDK-8141123 Change int's to size_t in FreeIdSet

Tom Benson tom.benson at oracle.com
Wed Nov 25 21:33:39 UTC 2015


Hi Alex,
Looks good, but I just noticed one other minor detail:  I think the 
definition of 'res' at line 526 should also be uint, for consistency.  I 
don't need to see an updated webrev for that.
Tom

On 11/25/2015 4:09 PM, Alexander Harlap wrote:
> Here is new revision :
>
> http://cr.openjdk.java.net/~aharlap/8141123/webrev.03
>
> Alex
>
> On 11/25/2015 3:05 PM, Alexander Harlap wrote:
>> Thank you Tom.
>> Yes, you are right.
>>
>> So I will modify code.
>>
>> I need to recall back my message for sponsor to push JDK-8141123.
>>
>> We need first complete review process.
>>
>> Alex
>>
>> On 11/25/2015 1:41 PM, Tom Benson wrote:
>>> Hi Alex,
>>> I notice the the new code in claim_par_id seems to have a needless 
>>> conditional at 526:
>>>
>>>  521   while (_hd == end_of_list) {
>>>  522     _waiters++;
>>>  523     _mon->wait(Mutex::_no_safepoint_check_flag);
>>>  524     _waiters--;
>>>  525   }
>>>  526   if (_hd == end_of_list) {
>>>  527     return UINT_MAX;
>>>  528   } else {
>>>
>>> The old code used to break out of the loop if "safepoint" was true, 
>>> so the conditional was useful.
>>>
>>> Aside from that, the change looks OK to me.
>>> Tom
>>>
>>> On 11/20/2015 11:12 AM, Alexander Harlap wrote:
>>>> I changed title of CR.
>>>>
>>>> Alex
>>>>
>>>> On 11/19/2015 4:19 AM, Thomas Schatzl wrote:
>>>>> Hi,
>>>>>
>>>>> On Wed, 2015-11-18 at 21:37 +0100, Thomas Schatzl wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Mon, 2015-11-16 at 13:25 -0500, Alexander Harlap wrote:
>>>>>>> Here is another revision,
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~aharlap/8141123/webrev.02/
>>>>>>>
>>>>>>> It has more cleanup.
>>>>>>>
>>>>>>> Version was tested by RBT.
>>>>>>>
>>>>>    please change the subject of the CR, as it is more a general 
>>>>> cleanup
>>>>> of the code now.
>>>>> Alternatively you may split this change into two CRs, one just 
>>>>> removing
>>>>> code, the other changing types.
>>>>>
>>>>> I am fine with either.
>>>>>
>>>>> Thanks,
>>>>>    Thomas
>>>>>
>>>>>
>>>>
>>>
>>
>




More information about the hotspot-gc-dev mailing list