RFR: 8343893: Test jdk/jfr/event/runtime/TestNativeMemoryUsageEvents.java failed: heap should have grown and NMT should show that: expected 0 > 0 [v4]

[Gerard Ziemski]

On Mon, 18 Nov 2024 17:03:54 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> # Background
>> 
>> With the implementation of [JDK-8312132](https://bugs.openjdk.org/browse/JDK-8312132) we added the `MemoryFileTracker` (MFT). Unfortunately, this work failed to implement JFR integration. This, in turn, meant that (generational) ZGC has not correctly reported its heap usage via the NMT JFR events. We (as in Oracle) never saw this issue in testing, as we didn't run this test for all possible GC configurations. With an update of our test configurations this is no longer the case: We run this test for all possible GCs and the test now fails for generational ZGC.
>> 
>> # The fix
>> 
>> I implemented JFR events for the MFT by adding all of its reserved and committed memory into the JFR data, similarly to the `VirtualMemoryTracker`. I also added an explicit `run` using `ZGC` to the failing test, to ensure that any actual regressions are found.
>
> Johan Sjölen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove ZGC run test

Just and observation - you used here the acronym `MFT` and it just jumped out at me, then I looked at `memoryFileTracker` and it feels inconsistent to me.

Did we ever consider naming it `nativeMemoryFileTracker`? Then we would have `NMFT`, which extends `NMT` better, IMHO.

Just something to keep in mind if we ever feel like cleaning up the names in `NMT` area.

LGTM

src/hotspot/share/nmt/nmtUsage.cpp line 105:

> 103:       _vm_total.committed += vm->committed();
> 104:     });
> 105:   }

I am missing something here, in memoryFileTracker.hpp you say:


// All memory is accounted as committed, there is no reserved memory.
// Any reserved memory is expected to exist in the VirtualMemoryTracker.


but here we use MFT to account for both reserved and committed?

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

PR Review: https://git.openjdk.org/jdk/pull/22204#pullrequestreview-2446030052
Marked as reviewed by gziemski (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22204#pullrequestreview-2446053916
PR Review Comment: https://git.openjdk.org/jdk/pull/22204#discussion_r1848701088