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

[Thomas Stuefe]

On Mon, 20 Mar 2023 17:23:03 GMT, Justin King <jcking at openjdk.org> wrote:

> > Your proposal has costs:
> > 
> > * the inherent risk of writing over malloc-allocated boundaries ("Any implementation returning an invalid size from malloc_usable_size is a broken implementation and has no business existing or being supported" sounds good and all but customers with crashing systems won't care if it's us screwing up or the libc maintainers).
> 
> But that is the case for all of libc. `malloc_usable_size` is no different.
> 
> > * The loss of precision of NMT-provided overwrite checks
> 
> I fail to see how this is the case. The idea here is allocate at least N, then have `malloc` tell how much it actually gave `M`. M is the actual size of the allocation and what you want. This wouldn't be used except for arena-like implementations or maybe containers where extra size can be used, not fixed size C++ objects.

Okay, I see what you did there. The trailing canary is now placed either after the excess size or the user requested size depending on whether the output parameter "allocated_size" was given or not. Still, not a big fan. Tying a difference in mechanics to whether or not an output receptor var had been given is at least surprising.

> 
> > * increased complexity and reviewer churn. There is a ton of things to do and little reviewer time to go around.
> 
> Sure, but adding simple size feedback and completely rewriting Arena to use Metaspace allocator are two different things, with the latter being much more complicated.

Arena coding is decades old (to my knowledge) and, as you have probably noticed, not that awesome. It is also full of pitfalls. I have spent time reworking it, and it needs a lot of care to not break anything in hotspot. My foot is full of holes already. So it also needs a lot of thorough reviewers. Thankfully we managed to reduce the complexity of Arenas already by ripping out UseMallocOnly mode.

Time spend reviewing this code could be spent improving e.g. Metaspace, and we'd arrive ultimately at a better solution and have less code to maintain.

> 
> > Do the benefits outweigh these costs? Maybe, but we don't know that since there are no numbers. What we do know is that most small-grained allocations - that could benefit from your change most - are C++ objects with fixed sizes. Leaves those little raw buffers in things like stringStream, and arena chunks.
> 
> C++ objects with fixed sizes are not the target here. Only allocations which can make use of the extra space, like arenas or contiguous containers.

Which is why I had been asking for numbers. How much memory is actually to be gained by this change?

After many years of supporting the JDK on the weirdest platforms I am very hesitant to overwrite malloc boundaries on the promise that this would be just fine. It is a risk, despite all you write, and it must be worth it. A few KB? Not worth it. A few dozen MB? That would be more interesting.

> 
> > But for arena chunks in particular, it makes sense to discuss alternatives. E.g. we could just re-use the metaspace allocator for hs arenas. Metaspace is a much better arena allocator than hs arenas, which makes sense since Metaspace is the spiritual successor.
> > A variant of that would be to just roll our own allocator for arenas. Allocate chunks via os::reseve/commit_memory, and take care of committing/uncommitting ourselves. That would be trivial to implement.
> > Both examples would solve not only the overhead-per-chunk problem but also the retention problem in glibc and other libc implementations (e.g. AIX, Solaris), which I see as the larger problem.
> 
> Doesn't the metaspace allocator have extra logic for garbage collection and limits on the maximum size of an allocation? Its unlikely to out perform a simple arena allocator which allocates chunks of memory and simply increments a pointer in most cases to dole out memory. Additionally all allocations in the arena are extremely short lived typically. Those that are not are very small, such as the arena used for JNI handles.

Metaspace is an arena allocator. It is geared toward very fast allocation of tiny things and fast bulk release. In contrast to Hotspot Arenas it mmaps its chunks, has an effective free chunk management, and returns memory to the OS much more willingly than libc does. It also can deal with premature deallocation. The ties to GC are slim and could be factored out. I need to do this for Lilliput anyway.

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

PR Comment: https://git.openjdk.org/jdk/pull/13081#issuecomment-1477929034