RFR (L) 8028541: Native Memory Tracking enhancement

[Zhengyu Gu]

Actually, I am working on this.

It was caught in between outputStream's format check and Mikael's 
macros, I need to merge up the repo for this.

There are a lot more instances in memReporter.cpp.

Thanks,

-Zhengyu

On 6/12/2014 10:43 PM, Coleen Phillimore wrote:
>
> 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
>>>>
>>>
>>
>