RFR: 8337217: Port VirtualMemoryTracker to use VMATree [v35]

[Afshin Zafari]

On Thu, 6 Mar 2025 18:38:20 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   review comments applied
>
> src/hotspot/share/nmt/virtualMemoryTracker.cpp line 107:
> 
>> 105:     //                str, NMTUtil::tag_to_name(tag), (long)reserve_delta, (long)commit_delta, reserved, committed);
>> 106:   };
>> 107: 
> 
> 8350567 is merged now! I think that that PR should be merged in.

Under test.

> src/hotspot/share/nmt/virtualMemoryTracker.cpp line 195:
> 
>> 193: bool VirtualMemoryTracker::print_containing_region(const void* p, outputStream* st) {
>> 194:   ReservedMemoryRegion rmr = tree()->find_reserved_region((address)p);
>> 195:   log_debug(nmt)("containing rgn: base=" INTPTR_FORMAT, p2i(rmr.base()));
> 
> Is this important?

Removed.

> src/hotspot/share/nmt/virtualMemoryTracker.cpp line 216:
> 
>> 214:   MemTracker::NmtVirtualMemoryLocker nvml;
>> 215:   tree()->visit_reserved_regions([&](ReservedMemoryRegion& rgn) {
>> 216:     log_info(nmt)("region in walker vmem, base: " INTPTR_FORMAT " size: %zu , %s, committed: %zu",
> 
> This should be `debug` level, or maybe even removed.

Removed.

> src/hotspot/share/nmt/virtualMemoryTracker.hpp line 35:
> 
>> 33: #include "utilities/ostream.hpp"
>> 34: 
>> 35: // VirtualMemoryTracker (VMT) is the internal class of NMT that only the MemTracker class uses it for performing the NMT operations.
> 
> `... uses it for ...`, delete "it" (grammar issue).

Removed.

> src/hotspot/share/nmt/virtualMemoryTracker.hpp line 41:
> 
>> 39: // state (reserved/released/committed) and MemTag of the regions before and after it.
>> 40: //
>> 41: // The memory operations of Reserve/Commit/Uncommit/Release (RCUR) are tracked by updating/inserting/deleting the nodes in the tree. When an operation
> 
> `(RCUR)` can be removed, it's never mentioned again.

Removed.

> src/hotspot/share/nmt/virtualMemoryTracker.hpp line 49:
> 
>> 47: //   - uncommitted size of a MemTag should be <= of its committed size
>> 48: //   - released size of a MemTag should be <= of its reserved size
>> 49: 
> 
> I don't believe that these are checked, right? So this can be deleted.

As said at the end of line, when they are applied to the VirtualMemorySummary they will be checked.
Is it OK if I limit/wrap the comments to 80-columns text?

> src/hotspot/share/nmt/virtualMemoryTracker.hpp line 132:
> 
>> 130:     }
>> 131:   }
>> 132: 
> 
> Dead code?

Removed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1984660439
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1984660659
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1984660818
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1984660197
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1984659994
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1984659801
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1984657977