RFR: JDK-8304421: Introduce malloc size feedback [v3]

[Justin King]

On Fri, 17 Mar 2023 18:58:10 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

> Hi,
> 
> some initial thoughts.
> 
> I find using excess memory in this manner quite scary. It depends on the implementation correctly not using what it reports. Eg glibc malloc_usable_size comes with a warning label of "this is bad practice". Practically speaking we now treat new paths, which are UB, and may trigger paths in libcs that are not well tested. The gains are probably not large enough to justify this risk.

Any implementation returning an invalid size from `malloc_usable_size` is a broken implementation and has no business existing or being supported.

glibc `malloc_usable_size` man pages have no such warning, they do have the following quote `Although the excess bytes can be overwritten by the application without ill effects, this is not good programming practice: the number of excess bytes in an allocation depends on the underlying implementation.` The warning is there so that callers do not rely on any specific size being returned other than it being at least the original request size. Two different allocations for the same size could have different excess bytes, so make no assumptions.

Size feedback is likely being added to operator new in the next version of C++ where a tuple will be returned containing the pointer and size, C has yet to follow suite but likely will in the future. At which point that could be used.

> 
> We could probably just resize arena chunks correctly to page sizes or power of two sizes; much simpler that way.

That would not work and would still have waste. It is heavily dependent on the underlying `malloc()` implementation which is free to change. For example if you request exactly a page from glibc `malloc()` your're likely going to get more than that. Whether power-of-two will work also depends on the implementation.

> 
> Lunping refactoring together with functional changes does not help. Makes it harder to spot the functional changes. Refactoring can be done in separate RFEs if necessary. Since refactoring is often a matter of opinion, it's also fairer to discuss them separately from functional changes.
> 
> More thoughts:
> 
> * It is unspecified what the effect should be on NMT allocation guards. Would the guard be added to the user-requested size or to the actual size? The former would render this improvement pointless; the latter makes me personally unhappy since we lose our ability to catch off by one overwrites. This needs to be specified, and we need regression tests for this.

Its the latter. The size is now the actual size. The point of size feedback is "allocate at least N bytes, then tell me how much you actually allocated".  I'll work on adding some tests.

> * Speaking of it, there are no tests.
> * The duplication of code in NMTPreInit is just plain bad, as is the duplication of os::free in os.cpp. That said, I don't get the point of free_sized. Why is this needed?

free_sized is in C23 and is an optimization for many `malloc()` implementations that allows free to be faster. It's the same rationale as having sized delete in C++. If the size is known, the implementation can skip having to determine the size in order to do the free.

That said I could probably try to combine them into a single implementation under the hood.

> * Platform-dependent changes should go into their respective platform folders
> * Does muslc support malloc_usable_size? If not, pls make your code conditional for glibc (see e.g. the trim coding in the linux branch)

Yes it supports it.

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

PR: https://git.openjdk.org/jdk/pull/13081