Code review request: JDK-8001592 NMT: assertion failed: assert(_amount >= amt) failed: Just check: memBaseline.hpp:180

[Coleen Phillimore]

The comment in memSnapshot.hpp still refers to the arena_memory record 
as an arena_size record.   I do like arena_memory_record better because 
it means the memory that the Arena points to (which is allocated 
separately).

*+      // arena size record is a special case, which we have to compare*
*+      // sequence number against its associated arena record.
...
**+       // 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.*


Other than this, it looks good.

Coleen

On 11/9/2012 1:36 PM, Zhengyu Gu wrote:
> Updated webrev based on Karen and Coleen's comments.
>
> http://cr.openjdk.java.net/~zgu/8001592/webrev.01/
>
> Thanks,
>
> -Zhengyu
>
> On 11/8/2012 11:19 AM, Karen Kinnear wrote:
>> Zhengyu,
>>
>> Code looks good. Thank you for the fixes.
>>
>> Minor comments:
>> 1. memBaseline.cpp line 119 anonymouse ->  anonymous
>>
>> 2. Much prefer the new MallocRecordIterator::current() behavior!!!
>>
>> 3. MallocRecordIterator::next()
>> Can you possibly clarify? Lines 194-195 say that if there is an 
>> associated
>> arena record it has to be the previous because of sorting order?
>>   -- what about sorting order means it is previous?
>>   - and it appears that you are looking at next_rec relative to 
>> prev_rec. What about
>>     the current rec - isn't that in the middle?
>>
>> - and many thanks for the detailed code comments and improved 
>> variable names.
>>
>> thanks,
>> Karen
>>
>>
>> On Nov 7, 2012, at 11:26 AM, Zhengyu Gu wrote:
>>
>>> The assertion failure reflects that total memory that backs arenas 
>>> are greater than total memory chunks for backing all arenas, which 
>>> is obviously incorrect.
>>>
>>> The problem is due to Arena object, although it is declared as 
>>> C-heap object, but it is used as value objects and stack objects as 
>>> well. Value and stack objects do not have tracking records in NMT, 
>>> which can leave arena size records alone in NMT. Because NMT uses 
>>> Arena's deallocation records to cleanup size records, that means 
>>> those leftover size records are not get cleanup.
>>>
>>> The solution is to use Arena's destructor to reset arena size to 0, 
>>> and NMT uses this record to remove the corresponding record.
>>>
>>> The webrev also cleanup Memsnapshot::merge() routine, since virtual 
>>> memory records now are stored in separate array, malloc staging area 
>>> can keep only one record for each address (whoever has higher 
>>> sequence number win), which should reduce memory usage by NMT.
>>>
>>> Webrev: http://cr.openjdk.java.net/~zgu/8001592/webrev.00/
>>>
>>> Tests:
>>>   vm.quick.testlist on Linux 32, Windows x64, Solaris AMD64 and Sparcv9
>>>   JPTR tests
>>>
>>>
>>> Thanks,
>>>
>>> -Zhengyu