RFR: 8298992: runtime/NMT/SummarySanityCheck.java failed with "Total commi…tted (MMMMMM) did not match the summarized committed (NNNNNN)

[Gerard Ziemski]

On Wed, 16 Aug 2023 12:18:35 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

> During exhaustive tests, it is observed that during taking snapshot of NMT metrics it is possible that new allocations happen concurrently, although a `ThreadCritical` is used during copying current metrics to the snapshot.
> A loop is surrounding the copying and checks whether the copied and original are the same.

Changes requested by gziemski (Committer).

The loop in `copy_to` seems to only exit if no-one allocated memory while the loop was busy. Should we consider adding some max loop counter to bail out if some threshold was reached?

I am a bit uncomfortable with a loop here that in theory might never terminate even if in practice your testing reveals no issue. Just wanted to raise this issue, not 100% sure it needs to be addressed.

src/hotspot/share/services/mallocTracker.hpp line 194:

> 192:     // buffer in make_adjustment().
> 193:     ThreadCritical tc;
> 194:     size_t total_mallocs;

Can we rename from `total_mallocs` to `total_size`? When I looked at the code at first I thought we were counting here the number of malloc operations.

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

PR Review: https://git.openjdk.org/jdk/pull/15306#pullrequestreview-1587528217
PR Comment: https://git.openjdk.org/jdk/pull/15306#issuecomment-1686749594
PR Review Comment: https://git.openjdk.org/jdk/pull/15306#discussion_r1300433013