RFR: JDK-8318636: Add jcmd to print annotated process memory map [v6]

[Thomas Stuefe]

On Tue, 31 Oct 2023 17:08:15 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fix various builds
>
> src/hotspot/share/nmt/memMapPrinter.cpp line 79:
> 
>> 77:   const void* const min = MAX2(from1, from2);
>> 78:   const void* const max = MIN2(to1, to2);
>> 79:   return min < max;
> 
> I had to rewrite it as:
> 
> `return MAX2(from1, from2) < MIN2(to1, to2);`
> 
> to make sure I understand it, or better yet, why not have it as a macros?
> 
> `#define RANGE_INTERSECT(min, max) (return MAX2(from1, from2) < MIN2(to1, to2))`
> 
>  MAX2 and MIN2 are macros already and we're not doing anything fancy here.

I'll do the former, that is clearer I agree, but leave the latter out (I assume with the macro you mean add it to globalDefenitions.hpp).

I fear that a lot of bikeshedding and general discussions would start, and to do it properly needs a bit more time. Because we really would benefit from having a nice templatized utility classes for ranges. Like MemRegion, but that one is tied to HeapWord* I think.

> src/hotspot/share/nmt/memMapPrinter.cpp line 89:
> 
>> 87:   // NMT deadlocks (ThreadCritical).
>> 88:   Range* _ranges;
>> 89:   MEMFLAGS* _flags;
> 
> Why did you decide to have two separate memory chunks?
> 
> I think I'd have used a struct to hold Range* and MEMFLAGS* and use that struct in a single array.

Two reasons:

- more condensed, needs less memory. MEMFLAGS is a byte, Range is 16 bytes. A combined structure would be 24 bytes with padding (since the structure needs to be pointer aligned). We'd loose 7 bytes.
- It is more cache friendly, hence faster. I need to iterate through all ranges, but only need to access the MEMFLAGS for the one range I find. By keeping the ranges condensed, more of them fit into a cache line, so iteration is faster.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16301#discussion_r1378475306
PR Review Comment: https://git.openjdk.org/jdk/pull/16301#discussion_r1378476876