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

[Johan Sjölen]

On Thu, 6 Mar 2025 14:26:56 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

>> - `VMATree` is used instead of `SortedLinkList` in new class `VirtualMemoryTracker`.
>>  -  A wrapper/helper `RegionTree` is made around VMATree to make some calls easier.
>>  - `find_reserved_region()` is used in 4 cases, it will be removed in further PRs.
>>  - All tier1 tests pass except this https://bugs.openjdk.org/browse/JDK-8335167.
>
> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review comments applied

A few more :-).

Btw, you probably do not want to integrate any of your other NMT PRs (unless they are necessary). Integrating them might cause merge conflicts.

I think we're almost done!!! Great job on this :D

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.

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?

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.

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

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.

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.

src/hotspot/share/nmt/virtualMemoryTracker.hpp line 132:

> 130:     }
> 131:   }
> 132: 

Dead code?

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

PR Review: https://git.openjdk.org/jdk/pull/20425#pullrequestreview-2665310971
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1983875040
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1983878610
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1983879989
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1983870666
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1983869375
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1983868809
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1983866253