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