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

[Joseph Provino]

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/20150618/5bd95e2d/attachment.htm>