RFR (L) 8028541: Native Memory Tracking enhancement

[Coleen Phillimore]

Zhengyu,

In nativeCallStack.cpp why do you need 
PRAGMA_FORMAT_MUTE_WARNINGS_FOR_GCC?  Can you change the print formats 
to keep gcc from complaining?

Coleen

On 6/12/14, 8:34 AM, Zhengyu Gu wrote:
> Coleen,
>
> Thanks for the review.
>
> On 6/11/2014 7:21 PM, Coleen Phillimore wrote:
>>
>> Hi Zhengyu,
>>
>> I have early feedback but I haven't looked at the new files yet. 
>> Nothing major.  I like the new code and I'm glad you could add NMT 
>> tracking to vmError reporting.
>>
>> http://cr.openjdk.java.net/~zgu/8028541/webrev.00/src/os/solaris/vm/os_solaris.cpp.udiff.html 
>>
>>
>> + bool os::unsetenv(const char* name) {
>> +  assert(name != NULL, "Null pointer");
>> +  return (::unsetenv(name) == 0);
>> + }
>> +
>>
>> Can you add this to os_posix.cpp instead?  Someday this duplicate 
>> code will be consolidated so it can start in posix even though it's 
>> not posix.
>>
> OK.  I assume that aix can also use os_posix functions, right?
>
>> http://cr.openjdk.java.net/~zgu/8028541/webrev.00/src/os/windows/vm/os_windows.cpp.udiff.html 
>>
>>
>> I thought the original comment was more descriptive.  Is it still a 
>> workaround?
>>
> I think we still need this workaround. Auto-merge overwrote 
> Christian's comment, I will restore.
>
>> http://cr.openjdk.java.net/~zgu/8028541/webrev.00/src/share/vm/runtime/arguments.cpp.html 
>>
>>
>> Can you factor lines 3596-3623 into a function like verify_nmt_flag?  
>> Is it that you already know what nmt level is used from the launcher 
>> and are just checking consistency and initializing here?  These 
>> cascading if() code checking options gets too long if you're not 
>> careful.
>
> Will do.
>>
>> In memReporter.hpp
>>
>> A short comment before each class what they are for and how you would 
>> get to this code would be nice.  I think they correspond to NMT 
>> settings (summary vs. detail).  Mem*DiffReporter reports the 
>> difference between the current memory tracked and a previous snapshot 
>> it appears.
> Will do.
>>
>>
>> In memReporter.cpp
>>
>> Lines 302-311, you could have two functions that does this same 
>> calculation for these values so that it doesn't get broken.  The same 
>> calculation appears 61 and 64.
>>
>> If you use size_t can you avoid eagerly scaling the amounts? 
>> amount_in_current_scale appears 66 times which is a lot.  Can you 
>> only call the scaling when you print the amounts?
> I think that is what I did.  All calculations are done in byte scale, 
> only converts to current scale when checks amount > 0 in current 
> scale, and print the number.
>
>> In MemTracker.hpp
>>
>> Line 109, I think you can assert that the JVM is single threaded.
>>
> What's the reliable method to check JVM single thread mode? I tried in 
> early NMT implementation, but it was not reliable.
>
> Thanks,
>
> -Zhengyu
>
>> That's as far as I got today.  This looks good!
>>
>> Coleen
>>
>> On 5/22/14, 3:19 PM, Zhengyu Gu wrote:
>>> This is significant rework of native memory tracking introduced in 
>>> earlier releases.
>>>
>>> The goal of this enhancement is to improve scalability, from both 
>>> tracking memory and CPU usage perspectives, so it can scale well 
>>> with increased memory allocation in large applications.
>>>
>>> The enhancement is mainly focused on malloc memory tracking, whose 
>>> activities are several magnitude higher than virtual memory, and was 
>>> the main bottleneck in early implementation.
>>>
>>> Instead of using book keeping records for tracking malloc 
>>> activities, new implementation co-locates tracking data along side 
>>> with user data by using a prefixed header. The header size is 8 
>>> bytes on 32-bit systems and 16 bytes on 64-bit systems, which ensure 
>>> that user data also align properly.
>>>
>>> Virtual memory tracking still uses book keeping records, and 
>>> ThreadCritical lock is always acquired to alter the records and 
>>> related data structures.
>>>
>>> Summary tracking data is maintained in static data structures, via 
>>> atomic operations. Malloc detail tracking call stacks are maintained 
>>> in a lock free hashtable.
>>>
>>> The key improvements:
>>>   1. Up-to-date tracking report.
>>>   2. Detail tracking now shows multiple call frames. Number of 
>>> frames is compilation time decision, currently default to 4.
>>>   3. Malloc tracking is lock free.
>>>   4. Tracking summary is reported in hs_err file when native memory 
>>> tracking is enabled.
>>>   5. Query is faster, uses little memory and need a very little 
>>> process.
>>>
>>> The drawback is that, malloc tracking header is always needed if 
>>> native memory tracking has ever been enabled, even after tracking is 
>>> shutdown.
>>>
>>> Impacts:
>>>   The most noticeable impact for JVM developers, is that Arena now 
>>> also take memory type as constructor parameter, besides the new 
>>> operators.
>>>      Arena* a = new (mtCode) Arena()  => Arena* a = new (mtCode) 
>>> Arena(mtCode)
>>>
>>> The webrev shows modification of about 60 files, but most of them 
>>> are due to tracking API changes, mainly due to tracking stack, now, 
>>> is an object, vs. a single pc.
>>>
>>> The most important files for this implementations are:
>>>
>>> memTracker.hpp/cpp
>>> mallocTracker.hpp/cpp and mallocTracker.inline.hpp
>>> virtualMemoryTracker.hpp/cpp
>>> mallocSiteTable.hpp/cpp
>>>
>>> allocationSite.hpp
>>> nativeCallStack.hpp/cpp
>>> linkedlist.hpp
>>>
>>>
>>> Tests:
>>>   - JPRT
>>>   - NMT test suite
>>>   - vm.quick.testlist
>>>   - Kitchensink stability test for 16+ days
>>>   - FMW
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8028541
>>> Webrev: http://cr.openjdk.java.net/~zgu/8028541/webrev.00/
>>>
>>> Thanks,
>>>
>>> -Zhengyu
>>>
>>
>