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

[Thomas Stuefe]

On Thu, 3 Dec 2020 08:48:13 GMT, Anton Kozlov <akozlov at openjdk.org> wrote:

>> The CDS changes in filemap.cpp look reasonable to me. Today this code is used on Windows only, but we are thinking of using it for all platforms (sometime in JDK 17). Do you think `os::protect_memory(base, size, os::MEM_PROT_RWX)` will work on all platforms?
>
>> Do you think os::protect_memory(base, size, os::MEM_PROT_RWX) will work on all platforms?
> 
> It looks so. Not sure about AIX, it ends up with ::mprotect, at least theoretically should be fine. On Linux and macOS without hardening it should be also OK. Without this patch, we don't support macOS hardened mode at all. But I have some private hacks for CDS with hardening, they reuse some of windows code for reading the archive content instead of mapping, so the future looks even more convenient.

Hi Anton,

Unfortunately I am not sure anymore that a separate API for reserving code is practical. See https://github.com/openjdk/jdk/pull/1153 (https://bugs.openjdk.java.net/browse/JDK-8256155). People want to be able to use large paged memory for code. Large paged memory gets allocated via os::reserve_memory_special(). 

Today we already split the API space into two groups: os::reserve_memory() and friends, and os::(reserve|release)_memory_special(). Adding an "executable" API group to that would multiply the number of APIs by two. I am afraid we are stuck with the exec flag on reserve and commit.

If you are interested, there was a lively discussion under https://github.com/openjdk/jdk/pull/1161 (https://bugs.openjdk.java.net/browse/JDK-8243315). Among other things it was discussed whether we should get rid of multi-page regions (mixing various page sizes). See Linux::reserve_memory_special_hugetlb_mixed. This would simplify coding.

Since I feel bad now for causing you work, I give up any opposition to extending the APIs with the exec parameter. So I had a closer look at your original change again:

https://github.com/openjdk/jdk/pull/294/commits/114d9cffd62cab42790b65091648fe75345c4533

I wonder whether we could simplify things, if we let go of the notion that the code heap gets only committed on demand. I do not know how MacOS memory overcommit works in detail. But on Linux, committing memory increases process footprint toward the commit charge limit, and may need swap space, but it does not increase RSS as long as the memory is not touched. I do not know how important on MacOS delaying memory commit really is. If its not important, then we could just do this:

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)

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.

> Two functions like reserve_memory and reserve_memory_aligned look excessive. ReservedSpace for some reason tries to use the unaligned version first and when it fails (how should it know the result should be aligned?), fallbacks to reserve_memory_alignment. https://github.com/openjdk/jdk/blob/master/src/hotspot/share/memory/virtualspace.cpp#L227. It will be more straightforward to ask for alignment from the start when it's required. I'm going to make alignment a parameter for reserve_memory as well, with the default value to be "no specific alignment", and to remove reserve_memory_aligned. It will simplify the implementation of reserve_executable_memory with alignment argument, and I hope to propose the suggested refactoring separately from this PR.
> 

This makes sense, but is outside the scope of this RFE.

In general, I think in the API we need a separation between page size and alignment (both have been confused in the past, see discussion under https://github.com/openjdk/jdk/pull/1161). But page size is irrelevant for reserve_memory - which reserves per default just with os::vm_page_size() - but for reserve_memory_special we should specify both.

Cheers, Thomas

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

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