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

[Anton Kozlov]

On Thu, 3 Dec 2020 18:10:18 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>>> Unfortunately I am not sure anymore that a separate API for reserving code is practical. See #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().
>> 
>> I think os::reserve_memory_special is substantially different, it does not allow commit/uncommit. https://github.com/openjdk/jdk/blob/master/src/hotspot/share/memory/virtualspace.cpp#L170 basically defines it this way. I'm mostly concerned with interface for executable memory with commit.
>> 
>>> 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.
>> 
>> It adds another API. And it's interesting that the only user of executable_memory are ReservedSpace and VirtualSpace. Removing executable argument from the rest of cases seems beneficial. I find it in somewhat more strict comparing with a boolean flag. But it's not really required for MAP_JIT.
>> 
>> From interface implementation point of view, it's convenient to have a single interface with as many parameters as possible, even excessive and unused. It will enable tricky cases handling inside the implementation. executable_memory API is artificial separation in this case. It will be necessary if some combination of parameters are impossible to implement, but it's not our case, so we can live without it.
>> 
>>> I wonder whether we could simplify things, if we let go of the notion that the code heap gets only committed on demand. 
>> 
>> I'm not sure, what is the aim of the simplification below? Now access to uncommitted memory will cause a trap, just like on other platforms.
>> 
>>> 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.
>> 
>> madvise(FREE) is not sufficient unfortunately. For executable memory, it's best we can do. But we should not use it for regular memory.
>> 
>>> 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 #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.
>> 
>> If we talk about reveting to 114d9cf, there is no change beyond extra boolean argument, right?
>
>> > Unfortunately I am not sure anymore that a separate API for reserving code is practical. See #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().
>> 
>> I think os::reserve_memory_special is substantially different, it does not allow commit/uncommit. https://github.com/openjdk/jdk/blob/master/src/hotspot/share/memory/virtualspace.cpp#L170 basically defines it this way. I'm mostly concerned with interface for executable memory with commit.
> 
> That was not my point.
> 
> So you introduce os::reserve_executable_memory() - a new API which allocates small paged executable memory. If you want large paged executable memory you still need to call os::reserve_memory_special, which still needs to retain its executable parameter. So this new API does not cover all use cases for executable. Unless you also add a new os::reserve_executable_memory_special(). This is what I meant with doubling the API numbers.
> 
>> And it's interesting that the only user of executable_memory are ReservedSpace and VirtualSpace. Removing executable argument from the rest of cases seems beneficial. I find it in somewhat more strict comparing with a boolean flag. But it's not really required for MAP_JIT.
> 
> I'd like to keep the os::xxx APIs indepent from their use cases in ReservedSpace. In other words, they should stay consistent in themselves.
> 
>> 
>> From interface implementation point of view, it's convenient to have a single interface with as many parameters as possible, even excessive and unused. It will enable tricky cases handling inside the implementation. executable_memory API is artificial separation in this case. It will be necessary if some combination of parameters are impossible to implement, but it's not our case, so we can live without it.
>> 
>> > I wonder whether we could simplify things, if we let go of the notion that the code heap gets only committed on demand.
>> 
>> I'm not sure, what is the aim of the simplification below?
> 
> To remove the coding depending on executable-ness from commit and uncommit.
> 
>> 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?
> 
> 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
> 
> On commit, the OS makes sure the memory is underlayed with swap space. The memory also counts toward the commit charge of the process. What effects this has in practice highly depends on the OS. Hence my question. For the code heap, it may not matter that much.
> 
> And ignoring uncommit for executable memory was based on your observation that it did not show observable effects anyway for you, and that code heap does not shrink.
> 
>> 
>> > 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.
>> 
>> 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?
> 
>> 
>> > 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 #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.
>> 
>> If we talk about reveting to [114d9cf](https://github.com/openjdk/jdk/commit/114d9cffd62cab42790b65091648fe75345c4533), there is no change beyond extra boolean argument, right?
> 
> Yes, I think so.

> > > 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.

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

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