RFR: 8315362: NMT: summary diff reports threads count incorrectly [v2]

[Gerard Ziemski]

On Mon, 2 Oct 2023 09:19:54 GMT, Evgeny Ignatenko <duke at openjdk.org> wrote:

>> 8315362: NMT: summary diff reports threads count incorrectly
>
> Evgeny Ignatenko has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review comments

Looks good, thank you for fixing this.

I do see 2 issues, one needs to be fixed, the other is optional.

Did anyone ran this through Mach5 tests? At least tiers 1 through 5?

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

> 87:   }
> 88: }
> 89: 

I would prefer more explicit code here with respect to `_thread_count`:


void MallocMemorySnapshot::snapshot_thread_count() {
  if (ThreadStackTracker::track_as_vm()) {
    _thread_count = ThreadStackTracker::thread_count();
  } else {
    _thread_count = thread_count();
  }
}


But if we do that, the we should probably remove:

`    assert(_thread_count == 0, "_thread_count can not be used if ThreadStackTracker::track_as_vm() == false");
`

from `MallocMemorySnapshot::thread_count()`

Not going to insist on this change though.

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

> 155:   // Used only for ThreadStackTracker::track_as_vm() == true to report thread
> 156:   // count diff during "jcmd <pid> VM.native_memory summary.diff" correctly.
> 157:   int               _thread_count;

`_thread_count` should be of `size_t` type

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

Changes requested by gziemski (Committer).

PR Review: https://git.openjdk.org/jdk/pull/15490#pullrequestreview-1658339108
PR Comment: https://git.openjdk.org/jdk/pull/15490#issuecomment-1747518369
PR Review Comment: https://git.openjdk.org/jdk/pull/15490#discussion_r1346363171
PR Review Comment: https://git.openjdk.org/jdk/pull/15490#discussion_r1346360160