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

Zhengyu Gu zhengyu.gu at oracle.com
Mon Nov 5 10:01:29 PST 2012

Coleen,

Thanks for reviewing.

On 11/5/2012 12:26 PM, Coleen Phillimore wrote:

Zhengyu, + address committedrgnend = committedrgn->addr() + *! committedrgn->size(); * Should "end" be addr() + size() -1 ? I think we use this formula for end address. For example in stack calculation code, etc. In range checking, the end address is excluded, however.

Thanks,

-Zhengyu

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 containsxx to containxx is not correct. "contains" is the correct form to use, and "overlaps" rather than "overlap". Why the variable rename in VMMemPointerIterator::addreservedregion? 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