RFR: 8234930: Use MAP_JIT when allocating pages for code cache on macOS [v6]

[Anton Kozlov]

On Fri, 4 Dec 2020 12:15:33 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>>> > > reserve_memory :
>>> > > 
>>> > > * not executable: mmap MAP_NORESERVE, PROT_NONE
>>> > > * executable: mmap MAP_JIT _without_ MAP_NORESERVE, PROT_READ|PROT_WRITE|PROT_EXEC (so its committed and accessible right away)
>>> > > 
>>> > > commit_memory
>>> > > 
>>> > > * not executable: mmap without MAP_NORESERVE, PROT_READ|PROT_WRITE
>>> > > * executable: (return, nothing to do)
>>> > > 
>>> > > uncommit_memory
>>> > > 
>>> > > * not executable: mmap MAP_NORESERVE, PROT_NONE
>>> > > * executable: (return, nothing to do, since you indicate that this is that memory does not get returned to the OS immediately)
>>> > >
>>> > I'm not sure, what is the aim of the simplification [above]?
>>> 
>>> To remove the coding depending on executable-ness from commit and uncommit.
>> 
>> Sorry, how can e.g uncommit choose executable or not-executable case, if executable parameter is not provided?
>> 
>> ---
>> 
>>> > Now access to uncommitted memory will cause a trap, just like on other platforms.
>>> 
>>> Sorry, you lost me there. Where would I get a trap?
>> 
>> I mean, after a call to pd_uncommit_memory on linux the memory mprotected with PROT_NONE. Any access to that region will generate a signal.
>> 
>> ---
>> 
>>> 
>>> My point was, for executable=true:
>>> 
>>> * on reserve, commit executable memory right away instead of on-demand committing later
>>> * on commit and uncommit, just do nothing
>> 
>> As far as I understand you propose to remove lines 2010 - 2014 https://github.com/openjdk/jdk/blob/114d9cffd62cab42790b65091648fe75345c4533/src/hotspot/os/bsd/os_bsd.cpp#L2010
>> 
>> but later you suggest (in different context, but the statement is correct)
>> 
>>> The range would still be accessible though, but combining that with mprotect(PROT_NONE) should take care of this
>> 
>> The current implementation does mprotect(NONE). madvice(FREE) is not accounted immediately, but this hint is better than nothing.
>> 
>> ---
>> 
>>> > > Furthermore, about uncommit: I wonder whether madvice(MADV_FREE) would alone be already sufficient to release the memory. I have no Mac and cannot test this. The range would still be accessible though, but combining that with mprotect(PROT_NONE) should take care of this. Then we could just in general avoid the mmap(MAP_NORESERVE|MAP_FIXED) call. Then we do not need the exec parameter for uncommit at all.
>>> > 
>>> > 
>>> > madvise(FREE) is not sufficient unfortunately. For executable memory, it's best we can do. But we should not use it for regular memory.
>>> 
>>> How so? What is the difference?
>> 
>> madvise is really a hint and doesn't have exact effect as a real uncommit. A real, immediately accountable uncommit is a https://github.com/openjdk/jdk/blob/114d9cffd62cab42790b65091648fe75345c4533/src/hotspot/os/bsd/os_bsd.cpp#L2015. As I cannot do this for executable memory, I use madvise as an alternative to nothing.
>
>> > > 
>> > > madvise(FREE) is not sufficient unfortunately. For executable memory, it's best we can do. But we should not use it for regular memory.
>> > 
>> > 
>> > How so? What is the difference?
>> 
>> madvise is really a hint and doesn't have exact effect as a real uncommit. A real, immediately accountable uncommit is a
>> 
>> https://github.com/openjdk/jdk/blob/114d9cffd62cab42790b65091648fe75345c4533/src/hotspot/os/bsd/os_bsd.cpp#L2015
>> 
>> . As I cannot do this for executable memory, I use madvise as an alternative to nothing.
> 
> I may have not been clear, sorry. My point was:
> 
> For uncommit, we seem to have the option of either:
> 
> 1) mmap(MAP_FIXED|MAP_NORESERVE, PROT_NONE)
> 2) madvise(MADV_FREE) + mprotect(PROT_NONE)
> 
> You do (1) for !exec, (2) for exec. Why?
> 
> Either (2) works - reclaims memory for the OS. Then it can be used in all cases, exec or !exec. The commit would be a simple mprotect(PROT_RW). For uncommit, we would need no exec parameter.
> 
> Or (2) does not work, as you claim. Then why bother at all?
> 
> Interestingly, your initial proposal would have resulted in the following sequence of calls when committing executable memory:
> anon_mmap() 	-> mmap(MAP_PRIVATE | MAP_NORESERVE | MAP_ANONYMOUS | MAP_JIT, PROT_NONE)
> commit_memory() -> mprotect(PROT_READ|PROT_WRITE|PROT_EXEC)
> Note how on commit_memory() we never clear the MAP_NORESERVE flag. And still commit works? And does not trap on access? Because if that works this is an indication that MAP_NORESERVE has no meaning on MacOS.
> 
> I found nothing about MAP_NORESERVE in the kernel source you posted [1], in the MacOS manpage for mmap [2] nor the OpenBSD mmap manpage [3]. MAP_NORESERVE is a non-Posix extension, so I wonder if it gets even honored on MacOS or if they just provided the flag to avoid build errors.
> 
> If MAP_NORESERVE has no meaning, we do not need to call mmap() for committing and uncommitting; mprotect, maybe combined with madvise(MADV_FREE), should suffice.
> 
> Thanks, Thomas
> 
> [1] https://github.com/apple/darwin-xnu/blob/master/bsd/kern/kern_mman.c#L227
> [2] https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/mmap.2.html
> [3] https://man.openbsd.org/mmap.2

> 1. mmap(MAP_FIXED|MAP_NORESERVE, PROT_NONE)
> 2. madvise(MADV_FREE) + mprotect(PROT_NONE)
> 
> Or (2) does not work, as you claim. Then why bother at all?

Right, (2) is an actual state. It does not work like we'd want to (at least not immediately accounted in RSS). But the OS provides this interface and claims to react in some way.

>      MADV_FREE        Indicates that the application will not need the information contained in this address range, so the pages may be reused right away.  The address range will remain valid.  This is used with madvise() system call.

It enables the OS release the memory sometime later, for example. In contrast, doing nothing will keep the memory that is garbage.

>  I wonder if [MAP_NORESERVE] gets even honored on MacOS

Right, it's defined and not used, the only occurrence is https://github.com/apple/darwin-xnu/blob/0a798f6738bc1db01281fc08ae024145e84df927/bsd/sys/mman.h#L116

The use of MAP_NORESRVE slipped in from the original BSD pd_commit_memory code. I can delete it, if it bothers.

> If MAP_NORESERVE has no meaning, we do not need to call mmap() for committing and uncommitting; mprotect, maybe combined with madvise(MADV_FREE), should suffice.

I could not follow, sorry. Why it's so?

Thanks,
Anton

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

PR: https://git.openjdk.java.net/jdk/pull/294