RFR (XXS): JDK-8085983 G1CollectedHeap::collection_set_iterate_from() has unused code and can be simplified

[Stefan Karlsson]

Hi Joe,

On 2015-08-05 18:28, Joseph Provino wrote:
> Latest webrev is here: 
> http://cr.openjdk.java.net/~jprovino/8085983/webrev.01

Is this the latest webrev? My comments have not been addressed in this 
patch.

Thanks,
StefanK

>
> joe
>
> On 6/18/2015 10:52 AM, Joseph Provino wrote:
>>
>> On 6/10/2015 12:46 PM, Stefan Karlsson wrote:
>>> Hi Joe,
>>>
>>> On 2015-06-08 21:16, Joseph Provino wrote:
>>>> One file.
>>>>
>>>> CR: https://bugs.openjdk.java.net/browse/JDK-8085983
>>>>
>>>> webrev: http://cr.openjdk.java.net/~jprovino/8085983/webrev.00
>>>
>>> Some comments:
>>>
>>> *+      guarantee(!cl->doHeapRegion(cur), err_msg("doHeapRegion() returned true"));*
>>>
>>> - You don't need err_msg if you don't have any format specifiers in 
>>> the error message.
>>>
>> I'll fix that.
>>
>>> - I would prefer if you didn't put the cl->doHeapReagion() calls 
>>> inside guarantees. It's too easy to incorrectly assume that the code 
>>> in the guarantee is only verification code, IMHO.
>>>
>> That's a good point.  Something like this:
>>
>>     bool ret = cl->doHeapRegion(r);
>>     guarantee(ret == false, "doHeapRegion() returned true");
>>
>> Or do you prefer !ret?
>>
>>> - Shouldn't this be an assert instead of a guarantee?
>> I don't know the rules there.  Seems like assert would be better
>> so it doesn't slow down the product build.
>>
>> thanks.
>>
>> joe
>>
>>>
>>> Thanks,
>>> StefanK
>>>
>>>>
>>>> passes jprt.
>>>>
>>>> thanks.
>>>>
>>>> joe
>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150812/534dedf1/attachment.htm>