RFR: JDK-8320061: [nmt] Multiple issues with peak accounting [v2]

[Johan Sjölen]

On Fri, 24 Nov 2023 16:48:22 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> There are multiple issues with peak printing.
>> 
>> 1) Peak values are not considered when deciding whether allocations fall above the scale threshold:
>> 
>> NMT omits printing information if the values, expressed with the current NMT scale, don't rise above 0. For example, `jcmd myprog VM.native_memory scale=g` only shows allocations above 1GB. However, we should also show information that had *historically* large values, even if their current value is small or even 0.
>> 
>> A typical example is compiler memory usage, which can reach large peaks during VM initialization and may fall to insignificant numbers afterward. We still want to see those peaks.
>> 
>> 2) We decided to make peak printing a release-build feature with [JDK-8317772](https://bugs.openjdk.org/browse/JDK-8317772), but peak printing for virtual memory is still debug-only
>> 
>> 3) There is a bug in that VirtualMemory::peak that causes the peak value to be wrong; it gets not updated from the actual peak but from the commit increase, so the value shown is too low if we committed in multiple steps. That can be simply observed by "largest_committed" being smaller than "committed", e.g.: `(mmap: reserved=1048576KB, committed=12928KB, largest_committed=64KB)`
>> 
>> 4) Finally, we really should have better regression tests for peak accounting.
>> 
>> ----
>> 
>> This patch fixes points (1)..(3).
>> 
>> The majority of changes affect test cases, though, because many tests expect NMT to print nothing for allocations that are close to or at zero. But since we now print historical peaks even if current values are low (which is the point of this patch) these tests break.
>> 
>> I found that we have a lot of duplicated coding in the tests and reduced that with some common utility classes.
>> 
>> Tests: 
>> - locally tested runtime/NMT and the full gtest suite on x64 release, fastdebug and x86 fastdebug.
>> - GHA
>
> Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - feedback johan
>  - Merge branch 'master' into JDK-8320061-NMT-do-not-omit-information-if-historic-peak-raises-above-scale-threshold
>  - JDK-8320061-NMT-do-not-omit-information-if-historic-peak-raises-above-scale-threshold

Yup, we're good! LGTM.

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

Marked as reviewed by jsjolen (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16675#pullrequestreview-1750701283