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

[Johan Sjölen]

On Tue, 23 Apr 2024 13:21:49 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

>> Johan Sjölen has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Remove faulty condition after removing merging
>>  - Add failing test case
>
> src/hotspot/share/nmt/nmtMemoryFileTracker.cpp line 45:
> 
>> 43: void MemoryFileTracker::allocate_memory(MemoryFile* device, size_t offset,
>> 44:                                             size_t size, MEMFLAGS flag,
>> 45:                                             const NativeCallStack& stack) {
> 
> indentation does not match with the line above.

Fixed

> src/hotspot/share/nmt/nmtMemoryFileTracker.cpp line 123:
> 
>> 121: void MemoryFileTracker::Instance::allocate_memory(MemoryFile* device, size_t offset,
>> 122:                                                       size_t size, MEMFLAGS flag,
>> 123:                                                       const NativeCallStack& stack) {
> 
> indentation ...

Fixed

> src/hotspot/share/nmt/nmtMemoryFileTracker.cpp line 128:
> 
>> 126: 
>> 127: void MemoryFileTracker::Instance::free_memory(MemoryFile* device, size_t offset,
>> 128:                                                   size_t size) {
> 
> indentation.

Fixed

> src/hotspot/share/nmt/nmtMemoryFileTracker.cpp line 138:
> 
>> 136: 
>> 137: void MemoryFileTracker::Instance::print_report_on(const MemoryFile* device,
>> 138:                                                       outputStream* stream, size_t scale) {
> 
> indentation.

Fixed

> src/hotspot/share/nmt/nmtMemoryFileTracker.cpp line 152:
> 
>> 150:       auto snap = snapshot->by_type(NMTUtil::index_to_flag(i));
>> 151:       auto current = device->_summary.by_type(NMTUtil::index_to_flag(i));
>> 152:       // PDT stores the memory as reserved but it's accounted as committed.
> 
> What does PDT stand for?

Fixed

> src/hotspot/share/nmt/nmtMemoryFileTracker.hpp line 49:
> 
>> 47: 
>> 48:   // Each device has its own memory space.
>> 49:   using DeviceSpace = VMATree;
> 
> `DeviceSpace` is used only 3 times in 2 methods in cpp file. `VMATree` is also used all around the code.
> `VMATree` is preferable then.

OK

> src/hotspot/share/nmt/nmtMemoryFileTracker.hpp line 62:
> 
>> 60:     MemoryFile(const char* descriptive_name)
>> 61:       : _descriptive_name(descriptive_name) {
>> 62:     }
> 
> can fit into 1 line.

Fixed

> src/hotspot/share/nmt/nmtTreap.hpp line 26:
> 
>> 24: 
>> 25: #ifndef SHARE_NMT_TREAP_HPP
>> 26: #define SHARE_NMT_TREAP_HPP
> 
> SHARE_NMT_NMTTREAP_HPP

Fixed

> src/hotspot/share/nmt/vmatree.cpp line 201:
> 
>> 199:   });
>> 200: 
>> 201:   AddressState prev = {A, stA}; // stA is just filler
> 
> `AddressState prev{A, stA};` would be like other instances of ctor. I.e., remove the `=` sign.

Fixed

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1577496651
PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1577496492
PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1577496379
PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1577495303
PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1577495232
PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1577495058
PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1577498708
PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1577498619
PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1577501137