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

[Jesper Wilhelmsson]

Looks good!
/Jesper

Sangheon Kim skrev den 13/2/15 22:11:
> Hi All,
>
> Sorry for sending many emails for this simple fix.
> After some discussion with StefanK, I decided to add a macro for this assert.
>
> Webrev:
> http://cr.openjdk.java.net/~sangheki/8073115/webrev.01/
>
> Thanks,
> Sangheon
>
> On 02/13/2015 09:41 AM, Sangheon Kim wrote:
>> 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
>>>
>>
>