RFR(xs): 8073115: assert(_covered_region.contains(p)) needs better error messages

[Sangheon Kim]

Hi StefanK,

Thanks for reviewing this and please see inline.

On 02/13/2015 01:07 AM, Stefan Karlsson wrote:
> Hi Sangheon,
>
> On 2015-02-13 07:54, Sangheon Kim wrote:
>> Hi All,
>>
>> Please review this small enhancement for better error messages when assert fails.
>> This change would be helpful forJDK-8071930  <https://bugs.openjdk.java.net/browse/JDK-8071930>  which is hard to reproduce.
>>
>> I will need a sponsor for this change.
>>
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8073115
>>
>> Webrev:
>> http://cr.openjdk.java.net/~sangheki/8073115/webrev.00/
>
> http://cr.openjdk.java.net/~sangheki/8073115/webrev.00/src/share/vm/gc_implementation/parallelScavenge/objectStartArray.hpp.udiff.html
>
> You are duplicating the same assert three times. Could you create a 
> new define, say #define assert_covered_region_contains(addr) ..., and 
> remove the duplication? This will also have the benefit of lowering 
> the line noise in the functions you change.
Basically I agree with you.
But in this case I think the benefit of macro is limited to reduce 3 
lines than current proposal.
Unless this macro moves to general header file, its usage would be limited.
So let me skip adding a new macro at this time.

Thanks,
Sangheon

>
> Thanks,
> StefanK
>
>> Testing:
>> JPRT
>>
>> Thanks,
>> Sangheon
>

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