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

[Anton Kozlov]

On 29.09.2020 09:53, Thomas Stüfe wrote:
> 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.
>
> 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.

Thanks for pointing, I'll take a deeper look. 

In general, I'm trying to avoid such kind of bookkeeping, because it is
overhead (in CPU and memory). An API change could render this overhead to 
nothing, and I think this is a good thing even at cost of more complicated 
API (not saying bad API :). I'm not sure we can always avoid complicated API.
It abstracts different operating systems and also needs to do that
efficiently.
I find malloc/free analogy is useful. Free don't need the size parameter because it is possible to rather efficiently store (small overhead) the size 
beside the allocated block. But I'm not sure our interface needs to provide
the same level of generality. All our users are nearby and wecan always fix 
them or interface.

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

Agree, I think it possible to do Mapping so it would have minimal runtime
overhead, so probably it will emerge sometime.

>> Do you think it will be a bad thing? I.e. to allow exec commit only in
>> exec reserved mapping? 
>>
> 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.

I would suggest a metaspace-code section beside here :)

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

The questions are related, I think. Semantically the new set of functions 
(code_reserve,...) will not be a big difference. But the specialized function
would allow defining a clean API that is tied to use-case. So there will be 
no code_reserve_at, code_attemp_reserve,.. 

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

Yes, this is a good point. I don't have an immediate answer. Mapping class 
could solve this. To some extend, code_reserve and friends could also do 
that, because it is harder to confuse code_{reserve,commit,uncommit} 
functions and general ones.

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

Excuse me for the long message. I'll take a look is it possible to retrieve 
the information in runtime and look for the options. I think it should be 
there something, but it depends on the possible overhead.
Thanks,
Anton