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

Joseph Provino joseph.provino at oracle.com
Wed Aug 5 16:28:06 UTC 2015


Latest webrev is here: 
http://cr.openjdk.java.net/~jprovino/8085983/webrev.01

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: <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/attachments/20150805/3e535309/attachment.html>


More information about the hotspot-gc-dev mailing list