RFR: 8315462: [REDO] runtime/NMT/SummarySanityCheck.java failed with "Total committed (MMMMMM) did not match the summarized committed (NNNNNN)" [v2]

[Thomas Stuefe]

On Wed, 18 Oct 2023 09:03:53 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

>> When taking snapshot of malloc entries, other threads may update the items that already copied so change their corresponding sum/count. To make the sum/count values be consistent with whatever copied, they are calculated during copy and then set explicitly if any mismatch detected.
>> The function body is also moved to cpp file.  
>> 
>> ### Tests
>> mach5 tiers 1-5 all platforms.
>
> Afshin Zafari has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits:
> 
>  - Merge branch 'master' into _8315462
>  - 8315462: [REDO] runtime/NMT/SummarySanityCheck.java failed with "Total committed (MMMMMM) did not match the summarized committed (NNNNNN)"

Good. Minor nits. Up to you if you change anything.

Cheers, Thomas

src/hotspot/share/services/mallocTracker.cpp line 76:

> 74:   }
> 75:   // ThreadCritical is used only for Virtual Memory de-/allocations.
> 76:   // So, it is possible that concurrent malloc/free happened during the copy to snapshot.

I'd shorten this to something like "malloc counters may be updated concurrently", that's succinct enough. Also, we may switch to mutexes at some point, like we should, then this commend would be outdated.

src/hotspot/share/services/mallocTracker.cpp line 79:

> 77:   if (s->_all_mallocs.size() != total_size || s->_all_mallocs.count() != total_count) {
> 78:     s->_all_mallocs.set_size_and_count(total_size, total_count);
> 79:   }

Why just not do it unconditionally?

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

Marked as reviewed by stuefe (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16237#pullrequestreview-1684510816
PR Review Comment: https://git.openjdk.org/jdk/pull/16237#discussion_r1363517490
PR Review Comment: https://git.openjdk.org/jdk/pull/16237#discussion_r1363518179