RFR: 8297718: Make NMT free:ing protocol more granular [v2]

[Thomas Stuefe]

On Tue, 6 Dec 2022 20:20:33 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

> I think we should have reused the original `MemTracker::record_free()` and modified it to take an additional argument, something like:
> 

<skip>

Matter of taste. I dislike this kind of coding, even though it is common in hotspot, where you have a compound function and a bunch of flags (often uncommented) fine-controlling its behavior. You then end up having to read the code to understand its behavior exactly.

I like it better to have simple prototypes, simple building blocks and compound actions, and if needed, expose more granular actions with equally simple prototypes. Like here, we have MemTracker::record_free - a compound action that 
- check NMT enabled
- resolve malloc header
- deaccount
- mark header as dead
- return header pointer

and then, we have "deaccount" and "mark header as dead" exposed as more granular action that can be used where you need finer control.

> 
> This is way we are re-using as much code as possible without needing to duplicate much of it.
> 

There should be little duplication now as well.

> But more importantly, we would **NOT** miss this code:
> 
> ```
>     if (!enabled()) {
>       return memblock;
>     }
> ```
> 
> As it is now, we are calling NMT code always, even when NMT is OFF.
> 
> You can of course add that check itself back to your re-implementation copy directly in `os::realloc()`, but like I said, we should just modify the original `MemTracker::record_free()` method (and possibly give it a more apt name now that we can skip the record_free portion of it) and re-use the already existing code.

You lost me here. Where would that be? Note that in os::realloc(), we are in the code path for NMT enabled already.

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

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