MemorySegment JVM memory leak

[Ahmed]

Dear Maurizio and Uwe,

Thank you all for clarification and feedback for this discussion and looking forward to have DONT_NEED Implemented in JVM.

The main reason developers will come to new classes of MemorySegment when dealing with very huge amount of data and we can't have a terabytes Of RAMs for caching/buffering in real application.

I do highly suggest to have also method for madvise(&db_map[i],4096,MADV_REMOVE). This way developers will have options of flexibility to decide when to keep cache/buffer for better performance.

As developers, some times you open a huge index for some background calculation and I already knew that I don't catching/buffering.

Regards, Ahmed.

-----Original Message-----
From: Maurizio Cimadamore <maurizio.cimadamore at oracle.com> 
Sent: Thursday, April 23, 2020 1:49 PM
To: Uwe Schindler <uschindler at apache.org>; 'Ahmed' <ahmdprog.java at gmail.com>; panama-dev at openjdk.java.net
Subject: Re: MemorySegment JVM memory leak

I agree with Uwe - the number you have to look at here is RES, not VIRT. 
64bit machines have plenty of virtual addressing space, so it makes sense for a process to reserve the address space in the eventuality that the entire file will, at some point, be accessed. This doesn't mean that 100G of file are residing into main memory. It merely means that there is a range of addresses which, if accessed, will cause some OS magic to occur, so that the contents of the file at that address will be transparently loaded into main memory.

I now believe that I understand why you were claiming that your second test also had the issue (which I wasn't able to reproduce): you were seeing an high virtual space usage, and you thought it was an issue; but in all practical purposes it isn't.

The problem with your original example was that, as more and more contents of the mapped file were accessed, the virtual space turned into "real" main memory - e.g. the OS loaded chunks of the file into main memory, and kept doing that to extremely uncomfortable levels. As Uwe said, the main cause of this is the OS "swappiness" level, e.g. the tendency of a system to swap out pages from main memory and put them back to disk so as to make more (memory) space available for other processes. In some popular Linux distros, including the one I have, this level is set very high, to 60. Lowering this value will cause the system to rely on swap space less, which should in turn increase 'aggressiveness' in removing unused memory pages of your file from main memory.

The madvise(DONT_NEED) trick is essentially a "kick" that we can give to the OS to signal that we really no longer need a mapped file page. This doesn't make the page unavailable for later references (this is why the virtual memory usage remains as it is) - this merely tells the OS to unload some pages from main memory and put them back into the file (causing more I/O activity, of course). Since MappedByteBuffer has a
load() operation which does the opposite (e.g. ensure that certain contents _are_ in main memory), it only seems fair to have a dual API point which attempts to mark mapped pages as unused.

Note that there's a big difference between DONT_NEED and REMOVE; in the first case, if you later on access the same contents, these will be made available to you (although at the cost of some page faults and IO operations). In the latter case, I believe, a portion of the mapping will actually be made unavailable for later accesses, which means subsequent read at "removed" addresses will see e.g. a 0. Doing such a thing will, in other words, affect the values that client of a memory segment will see upon accesses - it's not just about performances, so I don't think that such an API would make sense in this context.

Maurizio

On 23/04/2020 09:21, Uwe Schindler wrote:
> Hi,
>
> your screenshot is perfectly fine. What's you issue? It shows VIRT=100G (which is the amount of virtual memory assigned to the process; and more importantly this is not consuming any physical memory, unless its paged in). From the area I am coming from, this looks like a perfectly organized Elasticsearch server with 32 GiB physical RAM that has an 100 Gigabyte index open: How much memory is paged in can be seen in the columns for the file cache, but that's not process specific. A good operating system should always use as much physical memory for the file cache as its available. If you just read from those pages and don't write (Lucene is doing that), it's very easy for the OS to quickly free them if its required anywhere else. If you use the mmap for read/write things get more complicated, as uncommitted pages may need to be written to disk first (for that you can use force()).
>
> It looks like you have to understand what TOP shows, this is all sane. VIRT is virtual memory; the amount of ADRESS SPACE (not memory) assigned to the process. Address space is there enough, no need to argue.
>
> Uwe
>
> -----
> Uwe Schindler
> uschindler at apache.org
> ASF Member, Apache Lucene PMC / Committer Bremen, Germany 
> https://lucene.apache.org/
>
>> -----Original Message-----
>> From: Ahmed <ahmdprog.java at gmail.com>
>> Sent: Thursday, April 23, 2020 10:01 AM
>> To: 'Maurizio Cimadamore' <maurizio.cimadamore at oracle.com>; 'Uwe 
>> Schindler' <uschindler at apache.org>; panama-dev at openjdk.java.net
>> Subject: RE: MemorySegment JVM memory leak
>>
>> Hello,
>>
>> Coming back to Linux mmap
>>
>> The problem with madvise(&db_map[i],4096,MADV_DONTNEED); Will make 
>> the memory segment in OS cache/buffer. I did a lot of testing and the 
>> OS eat all memory and put it as cache/buffer and OS will never 
>> release it until process die.
>>
>> The attached screenshot explains very well.
>>
>> I don't think madvise(&db_map[i],4096,MADV_DONTNEED); Is the perfect 
>> solution for Linux. There is madvise(&db_map[i],4096,MADV_REMOVE); 
>> Which completely return the memory segment as a free memory which it is excellent.
>>
>> But again madvise(&db_map[i],4096,MADV_REMOVE); Is slow and severely 
>> impact performance.
>>
>> Maybe a good solution to make a queue Of segment pages and quietly 
>> free them by madvise(&db_map[i],4096,MADV_REMOVE);
>>
>> I completely understand OS to put segment in cache/buffer if process 
>> does not respect the OS page size. But honestly, I can't understand 
>> why OS keep the memory segment in cache/buffer after written to disk 
>> in mmap. The whole idea of mmap is to have a direct access and avoid 
>> cache/buffering of traditional read/write
>>
>> By the way, in MAC I did a quick test and mmap is great and politely 
>> mark memory segment to free pool after written to disk. Unfortunately 
>> I don't have access to any Solaris, but I am very curious to know how 
>> it behave there, very long time ago I remember some news about and 
>> perfect implementation mmap by Sun Microsystem that time.
>>
>> How java C source code implement this in mmap. Can you please share c 
>> source code of mmap usage ?.
>>
>> Regards, Ahmed Hamdallah.
>>
>> -----Original Message-----
>> From: Maurizio Cimadamore <maurizio.cimadamore at oracle.com>
>> Sent: Thursday, April 23, 2020 4:48 AM
>> To: Uwe Schindler <uschindler at apache.org>; 'Ahmed'
>> <ahmdprog.java at gmail.com>; panama-dev at openjdk.java.net
>> Subject: Re: MemorySegment JVM memory leak
>>
>>
>> On 23/04/2020 01:34, Maurizio Cimadamore wrote:
>>> No. But that doesn't seem to be a very important issue? If you don't 
>>> need the original safe segment, you can just ditch it and leave it 
>>> to the GC?
>> Ok, I see what you mean now.
>>
>> You basically want that calling close() on the unsafe segment calls 
>> the cleanup action (unmap) of the original segment. And, it seems you 
>> don't want to write native code for it either, so you have no way to 
>> write a Runnable that does what you want.
>>
>> When I issued the patch originally, I allowed the possibility of 
>> passing an AutoCloseable as a cleanup action (not a Runnable), which 
>> meant you could pass the _safe_ segment as the cleanup action, and 
>> the cleanup action would have been called regardless of confinement.
>>
>> During the review process, it was observed that this was perhaps a 
>> bit too magic - e.g. calling close() would leave the original segment 
>> in an unconsistent state (e.g. isAlive() == true, but memory freed).
>>
>> I'll think a bit more whether something like that could be achieved 
>> in a way that doesn't add too much complexity to the API, I think I 
>> see maybe a path to get there (or maybe is the late hour :-) ).
>>
>> Thanks
>> Maurizio
>