RFR: JDK-8281015: Further simplify NMT backend [v2]

Matthias Baesken mbaesken at openjdk.java.net
Wed Feb 23 15:53:54 UTC 2022


On Wed, 23 Feb 2022 07:22:07 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> NMT backend can be further simplified and cleaned out.
>> 
>> - some entry points require NMT_TrackingLevel as arguments, some use the global tracking level. Ultimately, every part of NMT always uses the global tracking level, so in many cases the explicit parameter can be removed and the global tracking level can be used instead.
>> - `MemTracker::malloc_header_size(level)` + `MemTracker::malloc_footer_size(level)` are fused into `MemTracker::overhead_per_malloc()`
>> - when adding to `MallocSiteTable`, caller gets back a shortcut to the entry. That shortcut is stored verbatim in the malloc header. It consists of two 16-bit values (bucket index and chain position). That tupel finds its way into many argument lists. It can be simplified into single 32-bit opaque marker. Code outside the MallocSiteTable does not need to know what it is.
>> - Currently, the `MallocHeader` class contains a lot of logic. It accounts (in constructor) and de-accounts (in `MallocHeader::release()`). It would simplify code if `MallocHeader` were just a dumb data carrier and the `MallocTracker` would do the actual work.
>> - `MallocHeader` can be simplified, almost all members made constant and modifying accessors removed.
>> - In some places we handle inputptr=NULL gracefully where we should assert instead
>> - Expressions like `MemTracker::tracking_level() != NMT_off` can be simplified to `MemTracker::enabled()`.
>> - MemTracker::malloc_base (all variants) can be removed. Note that we have MallocTracker::malloc_header, which achieves the same and does not require casting to the header.
>> 
>> Testing:
>> 
>> - GHAs
>> - manually ran NMT gtests (all NMT modes) and NMT jtreg tests on Ubuntu x64
>> - SAP nightlies ran through. Note that since 8275301 "Unify C-heap buffer overrun checks into NMT" NMT is enabled by default in debug builds, so it gets a lot more workout in tests now.
>> 
>> Note that I wanted to manually verify that the gdb "call pp" command still works in order to not break Zhengyu's recent addition, but found its already broken. I filed https://bugs.openjdk.java.net/browse/JDK-8281023 and am preparing a separate patch.
>
> Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains ten commits:
> 
>  - Zhengyus proposals
>  - fix build error after merge (need const variant of malloc_header())
>  - merge master
>  - pp should handle NULL correctly
>  - remove mostly unused MallocTracker accessors for header members
>  - Remove use of NMT level and simplify malloc+realloc+free
>  - dumb down malloc header
>  - mst bucket+pos=marker
>  - remove malloc_base

please check copyright years, e.g.  src/hotspot/share/services/memTracker.cpp   (still 2021).
Otherwise looks okay to me.

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

Marked as reviewed by mbaesken (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7283


More information about the hotspot-dev mailing list