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

[Gerard Ziemski]

On Thu, 1 Dec 2022 13:59:25 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> This PR avoids the double registration of a failed `os::realloc` in NMT by doing the NMT free protocol steps inline (asserting block integrity, marking of MallocHeader, and registrering results to statistics). This is done by creating a new struct `FreePackage` which includes all of the necessary data for registrering results.
>> 
>> I also did some light refactoring which I think makes the protocol clearer, these are done piece wise as separate commits for the reviewers' convenience.
>
> Johan Sjölen has updated the pull request incrementally with 10 additional commits since the last revision:
> 
>  - Add test that asserts block integrity after revival
>  - Make MallocHeader NONCOPYABLE
>  - Rest of it
>  - Adjust comments
>  - Put back comment, suitably rewritten
>  - Note reversibility of mark_block_as_dead
>  - Rename record_free_block to record_free
>  - Rename record_free to deaccount
>  - Move FreePackage into MallocHeader and rename
>  - Rename as revive, delete copy ctr

Just so I understand the motivation for this fix correctly - the "double registration of a failed os::realloc in NMT" does not actually mess up the memory calculations, correct? We do `record_free()` before the failed `realloc()` so from the memory statistics point of view, it is not an issue?

The flags issue that Thomas is referring makes me want to ask: are we really allowed to `realloc()` using different set of flags? It seems just wrong to me (something to look into later perhaps?).

So if that's the only issue, then we could have potentially just passed `header->flags()` to the "reviving" `MemTracker::record_malloc()` and be done?

I do appreciate an attempt to clean this area up though and that we only mark the old memory free, after we know that `realloc()` worked and I do think it's an improvement.

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

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