Code review request: NMT: assertion failed: assert(rec->addr() + rec->size() <= cur->base()) failed: Can not overlap in memSnapshot.cpp

Coleen Phillimore coleen.phillimore at oracle.com
Mon Nov 5 09:26:07 PST 2012


Zhengyu,

*+          address committed_rgn_end = committed_rgn->addr() +*
*!          _       committed_rgn->size(_);

*

Should "end" be addr() + size() -1 ?

This looks fine otherwise.   I like the renames that you did within the 
code you changed.

Thanks,
Coleen

On 11/2/2012 1:21 PM, Zhengyu Gu wrote:
> Updated the webrev based on David's comment.  Overlapping stacks is 
> tracked by 8001743.
>
> http://cr.openjdk.java.net/~zgu/8001591/webrev.01/
>
> Thanks,
>
> -Zhengyu
>
> On 10/31/2012 9:32 PM, David Holmes wrote:
>> On 1/11/2012 2:12 AM, Zhengyu Gu wrote:
>>> NMT did not allow overlapped commit on memory regions, which is
>>> incorrect. Committing overlapped memory regions should be allowed, as
>>> long as the regions are within a reserved region.
>>
>> So the overlapping stacks that were detected is perfectly valid?
>>
>>>
>>> http://cr.openjdk.java.net/~zgu/8001591/webrev.00/
>>
>> The renaming from contains_xx to contain_xx is not correct. 
>> "contains" is the correct form to use, and "overlaps" rather than 
>> "overlap".
>>
>> Why the variable rename in VMMemPointerIterator::add_reserved_region? 
>> Given it is initialized from current() then 'cur' seems quite 
>> acceptable. Otherwise maybe it is current() that has the wrong name? 
>> The renaming generates a lot of noise in the webrev - it is hard to 
>> see the actual substantive changes that were made.
>>
>> Cheers,
>> David
>>
>>
>>> Thanks,
>>>
>>> -Zhengyu


More information about the hotspot-dev mailing list