On Thu, 13 Jul 2023 18:56:56 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:

>> This is a continuation of https://github.com/openjdk/jdk/pull/10085. I 
>> closed https://github.com/openjdk/jdk/pull/10085 because it had accumulated 
>> too much comment history and got confusing. For a history of this issue, see 
>> previous discussions [1] and the comment section of 10085.
>>  
>> ---------------
>> 
>> This RFE adds the option to trim the Glibc heap periodically. This can 
>> recover a significant memory footprint if the VM process suffers from 
>> high-but-rare malloc spikes. It does not matter who causes the spikes: the 
>> JDK or customer code running in the JVM process.
>> 
>> ### Background:
>> 
>> The Glibc is reluctant to return memory to the OS. Temporary malloc spikes 
>> often carry over as permanent RSS increase. Note that C-heap retention is 
>> difficult to observe. Since it is freed memory, it won't appear in NMT; it 
>> is just a part of RSS.
>> 
>> This is, effectively, caching - a performance tradeoff by the glibc. It 
>> makes a lot of sense with applications that cause high traffic on the 
>> C-heap. The JVM, however, clusters allocations and often rolls its own 
>> memory management based on virtual memory for many of its use cases.
>> 
>> To manually trim the C-heap, Glibc exposes `malloc_trim(3)`. With JDK 18 
>> [2], we added a new jcmd command to *manually* trim the C-heap on Linux 
>> (`jcmd System.trim_native_heap`). We then observed customers running this 
>> command periodically to slim down process sizes of container-bound jvms. 
>> That is cumbersome, and the JVM can do this a lot better - among other 
>> things because it knows best when *not* to trim.
>> 
>> #### GLIBC internals
>> 
>> The following information I took from the glibc source code and 
>> experimenting.
>> 
>> ##### Why do we need to trim manually? Does the Glibc not trim on free?
>> 
>> Upon `free()`, glibc may return memory to the OS if:
>> - the returned block was mmap'ed
>> - the returned block was not added to tcache or to fastbins
>> - the returned block, possibly merged with its two immediate neighbors, had 
>> they been free, is larger than FASTBIN_CONSOLIDATION_THRESHOLD (64K) - in 
>> that case:
>>   a) for the main arena, glibc attempts to lower the brk()
>>   b) for mmap-ed heaps, glibc attempts to completely unmap or shrink the 
>> heap.
>> In both cases, (a) and (b), only the top portion of the heap is reclaimed. 
>> "Holes" in the middle of other in-use chunks are not reclaimed.
>> 
>> So: glibc *may* automatically reclaim memory. In normal configurations, with 
>> typical C-heap allocation granularity, it is unlikely.
>> 
>> To increase the ...
>
> Thomas Stuefe has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Bikeshed Trim log lines

Hi David,

> How have you determined all of the places that need 
> `NativeHeapTrimmer::SuspendMark`?
> 

We now avoid trimming if the VM is in or nearing a SafePoint, which takes care 
of the majority of phases where we don't want to trim. The rest of the 
suspensions was just based on my gut feeling: mallocs are usually spread out 
more, but some releases happen in bulk, and those are easily identifiable and 
have the SuspendMark.

When thinking this up, I considered a lot of alternatives, e.g., hooking into 
os::malloc/free and setting a dead mans timer each time to give the process a 
"cool down" after invocations - all in the assumption that mallocs are 
clustered. But that was too expensive - we did a lot of work to make the normal 
malloc paths with NMT=off very cheap - and also ineffective since we don't see 
malloc use from outside hotspot (JDK or third-party), and those are the main 
targets. 

To really catch every malloc, I even hooked into glibc malloc hooks, but that 
was horribly ugly and they removed the malloc hooks in recent glibc versions.

Bottom line, I think the current solution is a good compromise of complexity vs 
usefulness.

> Couple of minor comments below but otherwise this seems good. I'll take it 
> for a spin through our CI.
> 

Thank you!

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14781#issuecomment-1635314453

Reply via email to