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

[Sangheon Kim]

Hi StefanK, Bengt and Jesper,

Thank you for the reviewing this.

Sangheon


On 02/15/2015 11:12 PM, Bengt Rutisson wrote:
>
> Hi Sangheon,
>
> On 2015-02-13 22:11, Sangheon Kim wrote:
>> 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/
>
> Looks even better. :)
>
> Thanks,
> Bengt
>
>>
>> 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
>>>>
>>>
>>
>

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