RFR: 8312132: Add tracking of multiple address spaces in NMT [v49]

[Gerard Ziemski]

On Thu, 25 Apr 2024 10:43:22 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> src/hotspot/share/nmt/memTracker.cpp line 71:
>> 
>>> 69:     if (!MallocTracker::initialize(level) ||
>>> 70:         !VirtualMemoryTracker::initialize(level) ||
>>> 71:         !MemoryFileTracker::Instance::initialize(level) ||
>> 
>> Is there a way to hide the `instance` so that we could do:
>> 
>> `!MemoryFileTracker::initialize(level)
>> `
>> 
>> not
>> 
>> `!MemoryFileTracker::Instance::initialize(level)`
>> 
>> just like the other calls here? The instance is not needed here and just an implementation detail.
>
> We could invert the relationship such that the outer class is the `AllStatic` class and the inner class is the allocatable class. I'll look into it at a later stage as it's all a big renaming.
> 
> Personally, I don't mind the `::Instance` nomenclature to indicate that "this is the global instance that we're accessing". As long as we keep away from static, global singletons that we can't make many instances like VirtualMemoryTracker and MallocTracker are written, I'm a happy goose.

I agree with you actually, but the `Instance` jumps out at me and makes me wonder why we decided to use it, compared to the others, that are happy to be static classes.

We should have it all done same way. If you like to use `Instance`, then `VirtualMemoryTracker` and `MallocTracker` should use one as well (at some point later).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1581338808