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

Tue Sep 29 06:53:12 UTC 2020

Hi Anton,

On Mon, Sep 28, 2020 at 10:21 PM Anton Kozlov <akozlov at openjdk.java.net>
wrote:

> On Mon, 28 Sep 2020 11:51:24 GMT, Anton Kozlov <akozlov at openjdk.org>
> wrote:
>
> >> @tstuefe My patch to remove MAP_FIXED from the memory reservation path
> should make it possible to revert all the
> >> os::reserve_memory changes in this patch.
> >
> > Interesting, thanks! I haven't evaluated the interface, just because it
> seemed to be out of scope for the task. I'll
> > take a look and update the patch.
>
> Hi,
>
> > Can we on MacOS not use mmap at os::commit_memory, like we do on other
> platforms? In os::commit_memory, the exec
> > parameter exists but on MacOS it is ignored.
>
> Unfortunately, no. It's commit_memory uses MAP_FIXED that is incompatible
> with MAP_JIT. So it is impossible to commit
> executable mapping, as commit is always MAP_FIXED. This change proposes a
> kind of 2-step mapping: MAP_JIT in reserve
> and mprotect-without-mmap in commit.
>

Okay

>
> > We now have to specify "exec" as parameter to os::reverve_memory(). This
> is another new restriction - before, I could
> > theoretically reserve one mapping and commit various parts of it with
> and without exec flag. With this change, the
> > whole mapping has to be either exec or !exec.
>
> I think this is a major point in how the interface can be perceived. I
> assumed reserve to be semantically equivalent to
> mmap. And commit/uncommit only to control how much memory is actually
> allocated in the mmaping. E.g. commit could only
> follow reserve and commit could only refer to a (part of) memory reserved.
> So it would be natural for reserve to
> dictate exec or !exec mode for commit in its memory region.
>

In principle I agree with you. But os::reserve and friends are vaguely
defined wrappers around a number of quite different OS side APIs. Since
they have zero documentation and very little testing, it is everyones guess
what they actually should do. All we have are their names and the various
different implementations, which differ in a lot of details from one
another. I know this since a long time ago I implemented the AIX side of
this API and that was painful.

We have:
1 mmap on most Posix systems. An API with not a lot of restrictions, on the
surface; allows partly unmapping and uncommitting parts; allows to set exec
permissions on a per-page base. But seems not on all OSes.
2 VirtualAlloc on Windows. A stricter API. Only allows unmapping the whole
mapping as a whole.
3 SystemV shared mem. Also strict; also only allows total unmapping. On
Linux used for os::reserve_memory_special (no clue what special means) to
allocate large pages. On AIX, we use it to get 64K pages. On AIX, we
transparently switch between one or the other.

To me, 2 and 3 feel very similar, as opposed to 1.

On Linux and AIX, we dynamically switch between 1 and 3; on AIX we hide
that fact. You are right, there is a missing building block (a mapping
class) which would keep information about a mapping. Arguably,
ReservedSpace feels like it wants to be that building block, but it is
bloated and occupies itself with a lot of heap specific stuff.

On AIX, to get information at commit/release time about the mapping, we
keep such a mapping-related management record internally (struct vmembk_t,
see
https://github.com/openjdk/jdk/blob/70b0fccf79ac7193b36c49aff0286fdc09bb370c/src/hotspot/os/aix/os_aix.cpp#L1834)
- and retrieve it whenever someone hands in a pointer allocated with
os::reserve_memory, see:
https://github.com/openjdk/jdk/blob/70b0fccf79ac7193b36c49aff0286fdc09bb370c/src/hotspot/os/aix/os_aix.cpp#L2372
. This code stems from before SAPs involvement in OpenJDK, when we had no
way to change code upstream, and allowed us to hide AIX complications from
upper layers.

This is certainly not the best solution, but it is something you could do
too for MacOS. That would mean you do not have to change the uncommit API.

Of course, a better solution would be a cleanly defined interface, with a
Mapping class which defines properties of a mapping, well documented and
tested :/ but thats a large effort. The small cleanups that are done (most
recently Stefans removal of MAP_FIXED) are a step in that direction.

>
> Do you think it will be a bad thing? I.e. to allow exec commit only in
> exec reserved mapping? Apparently, there was no
> demand to do the opposite before. For that, it is still possible to
> reserve another !exec mapping beside.
>
>
Well, I played with the thought of placing code fragments in Metaspace.
Just an idea now, but being able to set exec permission on a range of
memory would make matters a lot easier. As would not having to care about
exec state of the underlying memory when uncommitting the memory.

> > Why does os::uncommit_memory() now require "exec" as parameter? I know
> why you do it, but semantically it has no
> > meaning. I should be able to uncommit memory without knowing the exact
> flags the mapping had been established with. So
> > now the user has to carry this information and provide it back to the
> API when uncommitting? Right now probably all
> > uncommits happen in areas where the exec information is implicit by its
> usage, but who says this is always the case?
>
> I would propose for reserve, commit, uncommit to take a uniform set of
> parameters to model an object of a (missing)
> "mapping" class, which would contain a meta-information about e.g.
> permissions. Now the meta-information is available
> in the context, as you correctly have noted, so we don't really need to
> introduce the real object and class.
>

But what is the contract here for uncommit (and commit actually)?

It looks like I can change exec permission per page in a mapping
(regardless of its commit state) and it may be able to do this on commit()
- but not on MacOS - but on uncommit, it is just a "remind me again what
flag I used for this mapping" kind of thing. What happens when I
accidentally get it wrong, or there is a mismatch? Something the OS or the
hotspot OS layer should know already.

>
> It is hard for me to imagine a case for "uncommit region regardless
> exec/!exec permission" that would really justify
> the need for such uncommit request.
>

Uncommit has nothing to do with the exec state of the memory. I should not
have to know at uncommit time. Like malloc and free: at free(), I should
not have to know the size of the allocation established with malloc. The
information should be already there in the lower layer. I should not have
to provide it.

> So I would propose to consider executable and nonexecutable mappings
> rather different, as 1) it is required by some
> platforms like macOS,

I could live with that. The platforms which have this restriction are MacOS
and AIX. The latter does not have explicit commit at all, so exec
permission is established at reserve time and cannot be changed later.

> and 2) it is supported by the fact that now we know the exec/!exec type of
> a mapping when we work
> with it (e.g. heap vs codecache).

ok

> To continue the trend of the cleanup and to highlight the difference, it
> is possible
> to create specialized code_reserve/commit/uncommit that will be
> implemented with usual reserve/... on all platforms
> except macOS, and latter to use MAP_JIT in the implementation.

Not totally opposed to this, but how is this different from passing a
parameter?

Which brings me to another thing, if we add exec to os::reserve_memory, why
not to os::reserve_aligned, os::attempt_reserve_at and
os::reserve_memory_special?

We had a similar thing some time ago when (Intel?) introduced the "reserve
heap on device" notion and pushed the "fd" parameter into os::reserve -
into some APIs, but not all, and Stefan Karlson just recently untangled
that mess a bit.

> Or I could stick with exec parameter. What do you think?
>
>
I think we agree that an overhaul would be good, but I do not want to block
you either.

I'd ask you to investigate first if the "exec" information can somehow be
obtained within os_bsd.cpp. Either by asking the platform (maybe there is
an api to get the flags a mapping had been established with?). Or, failing
that, by considering a similar solution as I have outlined above, like what
we did on AIX (could be done better than I did, with less code). Storing
mapping related information in a record and somehow keep it at OS level,
retrieving it as needed.

That would reduce the outside API changes by removing the need to pass in
exec to uncommit. Which really feels wrong to me.

(For the future, a better solution would be an os:: level class for
mappings. Maybe we should just develop a new clean interface beside the old
one and start migrating code to the new one.)

Cheers & thanks for your patience,

 Thomas

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