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

[Johan Sjölen]

On Tue, 23 Apr 2024 20:15:12 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> Johan Sjölen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Move TreapNode into Treap
>
> src/hotspot/share/nmt/memReporter.cpp line 914:
> 
>> 912:     MemoryFileTracker::Instance::print_report_on(dev, this->output(), scale());
>> 913:   }
>> 914: }
> 
> Does `devices.length()` and `devices.at(i)` are really needed to be exposed?
> 
> Can we consider pushing all this inside `MemoryFileTracker`?
> 
> We make 4 different API calls to MemoryFileTracker here in such a small function.

Sure, I prefer having all of the respective reporting code close to the classes instead of in the `memReporter.cpp` file. It'd be more in the current style to move the `print_report_on` method from `MemoryFileTracker` into `MemDetailReporter` instead.

> 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.

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

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