# HG changeset patch # User zgu # Date 1352507071 18000 # Node ID fb3190e77d3c58f13f9f400cf35d3cc52bec0545 # Parent 8940ddc1036f414b49444a9c37f150fade50237e 8001592: NMT: assertion failed: assert(_amount >= amt) failed: Just check: memBaseline.hpp:180 Summary: Fixed NMT that miscounted arena memory when it is used as value or stack object. Reviewed-by: acorn, coleenp diff -r 8940ddc1036f -r fb3190e77d3c src/share/vm/services/memBaseline.cpp --- a/src/share/vm/services/memBaseline.cpp Mon Nov 05 13:55:31 2012 -0800 +++ b/src/share/vm/services/memBaseline.cpp Fri Nov 09 19:24:31 2012 -0500 @@ -115,17 +115,25 @@ while (malloc_ptr != NULL) { index = flag2index(FLAGS_TO_MEMORY_TYPE(malloc_ptr->flags())); size_t size = malloc_ptr->size(); - _total_malloced += size; - _malloc_data[index].inc(size); - if (MemPointerRecord::is_arena_record(malloc_ptr->flags())) { - // see if arena size record present - MemPointerRecord* next_malloc_ptr = (MemPointerRecordEx*)malloc_itr.peek_next(); - if (MemPointerRecord::is_arena_size_record(next_malloc_ptr->flags())) { - assert(next_malloc_ptr->is_size_record_of_arena(malloc_ptr), "arena records do not match"); - size = next_malloc_ptr->size(); - _arena_data[index].inc(size); - used_arena_size += size; - malloc_itr.next(); + if (malloc_ptr->is_arena_memory_record()) { + // We do have anonymous arenas, they are either used as value objects, + // which are embedded inside other objects, or used as stack objects. + _arena_data[index].inc(size); + used_arena_size += size; + } else { + _total_malloced += size; + _malloc_data[index].inc(size); + if (malloc_ptr->is_arena_record()) { + // see if arena memory record present + MemPointerRecord* next_malloc_ptr = (MemPointerRecordEx*)malloc_itr.peek_next(); + if (next_malloc_ptr->is_arena_memory_record()) { + assert(next_malloc_ptr->is_memory_record_of_arena(malloc_ptr), + "Arena records do not match"); + size = next_malloc_ptr->size(); + _arena_data[index].inc(size); + used_arena_size += size; + malloc_itr.next(); + } } } malloc_ptr = (MemPointerRecordEx*)malloc_itr.next(); @@ -193,7 +201,7 @@ // baseline memory that is totaled over 1 KB while (malloc_ptr != NULL) { - if (!MemPointerRecord::is_arena_size_record(malloc_ptr->flags())) { + if (!MemPointerRecord::is_arena_memory_record(malloc_ptr->flags())) { // skip thread stacks if (!IS_MEMORY_TYPE(malloc_ptr->flags(), mtThreadStack)) { if (malloc_callsite.addr() != malloc_ptr->pc()) { diff -r 8940ddc1036f -r fb3190e77d3c src/share/vm/services/memPtr.hpp --- a/src/share/vm/services/memPtr.hpp Mon Nov 05 13:55:31 2012 -0800 +++ b/src/share/vm/services/memPtr.hpp Fri Nov 09 19:24:31 2012 -0500 @@ -165,7 +165,7 @@ return (flags & (otArena | tag_size)) == otArena; } - inline static bool is_arena_size_record(MEMFLAGS flags) { + inline static bool is_arena_memory_record(MEMFLAGS flags) { return (flags & (otArena | tag_size)) == (otArena | tag_size); } @@ -256,8 +256,8 @@ } // if this record records a size information of an arena - inline bool is_arena_size_record() const { - return is_arena_size_record(_flags); + inline bool is_arena_memory_record() const { + return is_arena_memory_record(_flags); } // if this pointer represents an address to an arena object @@ -266,8 +266,8 @@ } // if this record represents a size information of specific arena - inline bool is_size_record_of_arena(const MemPointerRecord* arena_rc) { - assert(is_arena_size_record(), "not size record"); + inline bool is_memory_record_of_arena(const MemPointerRecord* arena_rc) { + assert(is_arena_memory_record(), "not size record"); assert(arena_rc->is_arena_record(), "not arena record"); return (arena_rc->addr() + sizeof(void*)) == addr(); } diff -r 8940ddc1036f -r fb3190e77d3c src/share/vm/services/memSnapshot.cpp --- a/src/share/vm/services/memSnapshot.cpp Mon Nov 05 13:55:31 2012 -0800 +++ b/src/share/vm/services/memSnapshot.cpp Fri Nov 09 19:24:31 2012 -0500 @@ -50,7 +50,7 @@ tty->print_cr(" (tag)"); } } else { - if (rec->is_arena_size_record()) { + if (rec->is_arena_memory_record()) { tty->print_cr(" (arena size)"); } else if (rec->is_allocation_record()) { tty->print_cr(" (malloc)"); @@ -390,21 +390,31 @@ } } -void MemSnapshot::copy_pointer(MemPointerRecord* dest, const MemPointerRecord* src) { + +void MemSnapshot::copy_seq_pointer(MemPointerRecord* dest, const MemPointerRecord* src) { assert(dest != NULL && src != NULL, "Just check"); assert(dest->addr() == src->addr(), "Just check"); + assert(dest->seq() > 0 && src->seq() > 0, "not sequenced"); - MEMFLAGS flags = dest->flags(); + if (MemTracker::track_callsite()) { + *(SeqMemPointerRecordEx*)dest = *(SeqMemPointerRecordEx*)src; + } else { + *(SeqMemPointerRecord*)dest = *(SeqMemPointerRecord*)src; + } +} + +void MemSnapshot::assign_pointer(MemPointerRecord*dest, const MemPointerRecord* src) { + assert(src != NULL && dest != NULL, "Just check"); + assert(dest->seq() == 0 && src->seq() >0, "cast away sequence"); if (MemTracker::track_callsite()) { *(MemPointerRecordEx*)dest = *(MemPointerRecordEx*)src; } else { - *dest = *src; + *(MemPointerRecord*)dest = *(MemPointerRecord*)src; } } - -// merge a per-thread memory recorder to the staging area +// merge a recorder to the staging area bool MemSnapshot::merge(MemRecorder* rec) { assert(rec != NULL && !rec->out_of_memory(), "Just check"); @@ -412,71 +422,45 @@ MutexLockerEx lock(_lock, true); MemPointerIterator malloc_staging_itr(_staging_area.malloc_data()); - MemPointerRecord *p1, *p2; - p1 = (MemPointerRecord*) itr.current(); - while (p1 != NULL) { - if (p1->is_vm_pointer()) { + MemPointerRecord* incoming_rec = (MemPointerRecord*) itr.current(); + MemPointerRecord* matched_rec; + + while (incoming_rec != NULL) { + if (incoming_rec->is_vm_pointer()) { // we don't do anything with virtual memory records during merge - if (!_staging_area.vm_data()->append(p1)) { + if (!_staging_area.vm_data()->append(incoming_rec)) { return false; } } else { // locate matched record and/or also position the iterator to proper // location for this incoming record. - p2 = (MemPointerRecord*)malloc_staging_itr.locate(p1->addr()); - // we have not seen this memory block, so just add to staging area - if (p2 == NULL) { - if (!malloc_staging_itr.insert(p1)) { + matched_rec = (MemPointerRecord*)malloc_staging_itr.locate(incoming_rec->addr()); + // we have not seen this memory block in this generation, + // so just add to staging area + if (matched_rec == NULL) { + if (!malloc_staging_itr.insert(incoming_rec)) { return false; } - } else if (p1->addr() == p2->addr()) { - MemPointerRecord* staging_next = (MemPointerRecord*)malloc_staging_itr.peek_next(); - // a memory block can have many tagging records, find right one to replace or - // right position to insert - while (staging_next != NULL && staging_next->addr() == p1->addr()) { - if ((staging_next->flags() & MemPointerRecord::tag_masks) <= - (p1->flags() & MemPointerRecord::tag_masks)) { - p2 = (MemPointerRecord*)malloc_staging_itr.next(); - staging_next = (MemPointerRecord*)malloc_staging_itr.peek_next(); - } else { - break; - } + } else if (incoming_rec->addr() == matched_rec->addr()) { + // whoever has higher sequence number wins + if (incoming_rec->seq() > matched_rec->seq()) { + copy_seq_pointer(matched_rec, incoming_rec); } - int df = (p1->flags() & MemPointerRecord::tag_masks) - - (p2->flags() & MemPointerRecord::tag_masks); - if (df == 0) { - assert(p1->seq() > 0, "not sequenced"); - assert(p2->seq() > 0, "not sequenced"); - if (p1->seq() > p2->seq()) { - copy_pointer(p2, p1); - } - } else if (df < 0) { - if (!malloc_staging_itr.insert(p1)) { - return false; - } - } else { - if (!malloc_staging_itr.insert_after(p1)) { - return false; - } - } - } else if (p1->addr() < p2->addr()) { - if (!malloc_staging_itr.insert(p1)) { + } else if (incoming_rec->addr() < matched_rec->addr()) { + if (!malloc_staging_itr.insert(incoming_rec)) { return false; } } else { - if (!malloc_staging_itr.insert_after(p1)) { - return false; - } + ShouldNotReachHere(); } } - p1 = (MemPointerRecord*)itr.next(); + incoming_rec = (MemPointerRecord*)itr.next(); } NOT_PRODUCT(void check_staging_data();) return true; } - // promote data to next generation bool MemSnapshot::promote() { assert(_alloc_ptrs != NULL && _vm_ptrs != NULL, "Just check"); @@ -507,20 +491,25 @@ // found matched memory block if (matched_rec != NULL && new_rec->addr() == matched_rec->addr()) { // snapshot already contains 'live' records - assert(matched_rec->is_allocation_record() || matched_rec->is_arena_size_record(), + assert(matched_rec->is_allocation_record() || matched_rec->is_arena_memory_record(), "Sanity check"); // update block states - if (new_rec->is_allocation_record() || new_rec->is_arena_size_record()) { - copy_pointer(matched_rec, new_rec); + if (new_rec->is_allocation_record()) { + assign_pointer(matched_rec, new_rec); + } else if (new_rec->is_arena_memory_record()) { + if (new_rec->size() == 0) { + // remove size record once size drops to 0 + malloc_snapshot_itr.remove(); + } else { + assign_pointer(matched_rec, new_rec); + } } else { // a deallocation record assert(new_rec->is_deallocation_record(), "Sanity check"); // an arena record can be followed by a size record, we need to remove both if (matched_rec->is_arena_record()) { MemPointerRecord* next = (MemPointerRecord*)malloc_snapshot_itr.peek_next(); - if (next->is_arena_size_record()) { - // it has to match the arena record - assert(next->is_size_record_of_arena(matched_rec), "Sanity check"); + if (next->is_arena_memory_record() && next->is_memory_record_of_arena(matched_rec)) { malloc_snapshot_itr.remove(); } } @@ -528,17 +517,13 @@ malloc_snapshot_itr.remove(); } } else { - // it is a new record, insert into snapshot - if (new_rec->is_arena_size_record()) { - MemPointerRecord* prev = (MemPointerRecord*)malloc_snapshot_itr.peek_prev(); - if (prev == NULL || !prev->is_arena_record() || !new_rec->is_size_record_of_arena(prev)) { - // no matched arena record, ignore the size record - new_rec = NULL; - } + // don't insert size 0 record + if (new_rec->is_arena_memory_record() && new_rec->size() == 0) { + new_rec = NULL; } - // only 'live' record can go into snapshot + if (new_rec != NULL) { - if (new_rec->is_allocation_record() || new_rec->is_arena_size_record()) { + if (new_rec->is_allocation_record() || new_rec->is_arena_memory_record()) { if (matched_rec != NULL && new_rec->addr() > matched_rec->addr()) { if (!malloc_snapshot_itr.insert_after(new_rec)) { return false; diff -r 8940ddc1036f -r fb3190e77d3c src/share/vm/services/memSnapshot.hpp --- a/src/share/vm/services/memSnapshot.hpp Mon Nov 05 13:55:31 2012 -0800 +++ b/src/share/vm/services/memSnapshot.hpp Fri Nov 09 19:24:31 2012 -0500 @@ -31,7 +31,6 @@ #include "services/memBaseline.hpp" #include "services/memPtrArray.hpp" - // Snapshot pointer array iterator // The pointer array contains malloc-ed pointers @@ -165,39 +164,58 @@ }; class MallocRecordIterator : public MemPointerArrayIterator { - protected: + private: MemPointerArrayIteratorImpl _itr; + + public: MallocRecordIterator(MemPointerArray* arr) : _itr(arr) { } virtual MemPointer* current() const { - MemPointerRecord* cur = (MemPointerRecord*)_itr.current(); - assert(cur == NULL || !cur->is_vm_pointer(), "seek error"); - MemPointerRecord* next = (MemPointerRecord*)_itr.peek_next(); - if (next == NULL || next->addr() != cur->addr()) { - return cur; - } else { - assert(!cur->is_vm_pointer(), "Sanity check"); - assert(cur->is_allocation_record() && next->is_deallocation_record(), - "sorting order"); - assert(cur->seq() != next->seq(), "Sanity check"); - return cur->seq() > next->seq() ? cur : next; +#ifdef ASSERT + MemPointer* cur_rec = _itr.current(); + if (cur_rec != NULL) { + MemPointer* prev_rec = _itr.peek_prev(); + MemPointer* next_rec = _itr.peek_next(); + assert(prev_rec == NULL || prev_rec->addr() < cur_rec->addr(), "Sorting order"); + assert(next_rec == NULL || next_rec->addr() > cur_rec->addr(), "Sorting order"); } +#endif + return _itr.current(); } - virtual MemPointer* next() { - MemPointerRecord* cur = (MemPointerRecord*)_itr.current(); - assert(cur == NULL || !cur->is_vm_pointer(), "Sanity check"); - MemPointerRecord* next = (MemPointerRecord*)_itr.next(); - if (next == NULL) { - return NULL; + MemPointerRecord* next_rec = (MemPointerRecord*)_itr.next(); + // arena memory record is a special case, which we have to compare + // sequence number against its associated arena record. + if (next_rec != NULL && next_rec->is_arena_memory_record()) { + MemPointerRecord* prev_rec = (MemPointerRecord*)_itr.peek_prev(); + // if there is an associated arena record, it has to be previous + // record because of sorting order (by address) - NMT generates a pseudo address + // for arena's size record by offsetting arena's address, that guarantees + // the order of arena record and it's size record. + if (prev_rec != NULL && prev_rec->is_arena_record() && + next_rec->is_memory_record_of_arena(prev_rec)) { + if (prev_rec->seq() > next_rec->seq()) { + // Skip this arena memory record + // Two scenarios: + // - if the arena record is an allocation record, this early + // size record must be leftover by previous arena, + // and the last size record should have size = 0. + // - if the arena record is a deallocation record, this + // size record should be its cleanup record, which should + // also have size = 0. In other world, arena alway reset + // its size before gone (see Arena's destructor) + assert(next_rec->size() == 0, "size not reset"); + return _itr.next(); + } else { + assert(prev_rec->is_allocation_record(), + "Arena size record ahead of allocation record"); + } + } } - if (cur->addr() == next->addr()) { - next = (MemPointerRecord*)_itr.next(); - } - return current(); + return next_rec; } MemPointer* peek_next() const { ShouldNotReachHere(); return NULL; } @@ -213,9 +231,12 @@ // still chances seeing duplicated records during promotion. // We want to use the record with higher sequence number, because it has // more accurate callsite pc. -class VMRecordIterator : public MallocRecordIterator { +class VMRecordIterator : public MemPointerArrayIterator { + private: + MemPointerArrayIteratorImpl _itr; + public: - VMRecordIterator(MemPointerArray* arr) : MallocRecordIterator(arr) { + VMRecordIterator(MemPointerArray* arr) : _itr(arr) { MemPointerRecord* cur = (MemPointerRecord*)_itr.current(); MemPointerRecord* next = (MemPointerRecord*)_itr.peek_next(); while (next != NULL) { @@ -256,6 +277,12 @@ return cur; } + MemPointer* peek_next() const { ShouldNotReachHere(); return NULL; } + MemPointer* peek_prev() const { ShouldNotReachHere(); return NULL; } + void remove() { ShouldNotReachHere(); } + bool insert(MemPointer* ptr) { ShouldNotReachHere(); return false; } + bool insert_after(MemPointer* ptr) { ShouldNotReachHere(); return false; } + private: bool is_duplicated_record(MemPointerRecord* p1, MemPointerRecord* p2) const { bool ret = (p1->addr() == p2->addr() && p1->size() == p2->size() && p1->flags() == p2->flags()); @@ -348,8 +375,10 @@ DEBUG_ONLY( void dump_all_vm_pointers();) private: - // copy pointer data from src to dest - void copy_pointer(MemPointerRecord* dest, const MemPointerRecord* src); + // copy sequenced pointer from src to dest + void copy_seq_pointer(MemPointerRecord* dest, const MemPointerRecord* src); + // assign a sequenced pointer to non-sequenced pointer + void assign_pointer(MemPointerRecord*dest, const MemPointerRecord* src); bool promote_malloc_records(MemPointerArrayIterator* itr); bool promote_virtual_memory_records(MemPointerArrayIterator* itr); diff -r 8940ddc1036f -r fb3190e77d3c src/share/vm/services/memTracker.hpp --- a/src/share/vm/services/memTracker.hpp Mon Nov 05 13:55:31 2012 -0800 +++ b/src/share/vm/services/memTracker.hpp Fri Nov 09 19:24:31 2012 -0500 @@ -284,14 +284,14 @@ } } - // record arena size + // record arena memory size static inline void record_arena_size(address addr, size_t size) { - // we add a positive offset to arena address, so we can have arena size record + // we add a positive offset to arena address, so we can have arena memory record // sorted after arena record if (is_on() && !UseMallocOnly) { assert(addr != NULL, "Sanity check"); create_memory_record((addr + sizeof(void*)), MemPointerRecord::arena_size_tag(), size, - 0, NULL); + DEBUG_CALLER_PC, NULL); } }