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

[Evgeny Ignatenko]

On Wed, 4 Oct 2023 19:31:19 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> Evgeny Ignatenko has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Review comments
>
> 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.

Well in case you are not insisting I would like to leave this code as is :) 
Thank you for the review

> 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

Fixed. Thanks for catching this

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15490#discussion_r1346838076
PR Review Comment: https://git.openjdk.org/jdk/pull/15490#discussion_r1346836515