RFR: 8337217: Port VirtualMemoryTracker to use VMATree [v21]
Johan Sjölen
jsjolen at openjdk.org
Fri Feb 7 10:40:22 UTC 2025
On Thu, 6 Feb 2025 15:51:41 GMT, Afshin Zafari <azafari at openjdk.org> wrote:
>> - `VMATree` is used instead of `SortedLinkList` in new class `VirtualMemoryTrackerWithTree`.
>> - A wrapper/helper `RegionTree` is made around VMATree to make some calls easier.
>> - Both old and new versions exist in the code and can be selected via `MemTracker::set_version()`
>> - `find_reserved_region()` is used in 4 cases, it will be removed in further PRs.
>> - All tier1 tests pass except one ~that expects a 50% increase in committed memory but it does not happen~ https://bugs.openjdk.org/browse/JDK-8335167.
>> - Adding a runtime flag for selecting the old or new version can be added later.
>> - Some performance tests are added for new version, VMATree and Treap, to show the idea and should be improved later. Based on the results of comparing speed of VMATree and VMT, VMATree shows ~40x faster response time.
>
> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
>
> fixed merge problems
More things to be removed.
src/hotspot/share/nmt/vmatree.hpp line 215:
> 213: tty->print_cr("Flag %s R: " INT64_FORMAT " C: " INT64_FORMAT, NMTUtil::tag_to_enum_name((MemTag)i), tag[i].reserve, tag[i].commit);
> 214: }
> 215: }
This can be removed
src/hotspot/share/nmt/vmatree.hpp line 267:
> 265: });
> 266: tty->cr();
> 267: }
This can be removed, I'm rather sure(?)
src/hotspot/share/opto/stringopts.cpp line 173:
> 171: }
> 172: void add_control(Node* ctrl) {
> 173: assert(!_control.contains(ctrl), "only push once");
Remove the changes in this file.
src/hotspot/share/opto/stringopts.hpp line 1:
> 1: /*
Remove the changes in this file.
test/hotspot/gtest/nmt/test_nmt_memoryfiletracker.cpp line 46:
> 44: EXPECT_EQ(file->_summary.by_tag(mtTest)->committed(), sz(100));
> 45: tracker.free_memory(file, 50, 10);
> 46: EXPECT_EQ(file->_summary.by_tag(mtTest)->committed(), sz(90));
This change should be done in mainline, not in this PR.
test/hotspot/gtest/nmt/test_nmt_treap.cpp line 238:
> 236: EXPECT_LE(unexpected_count, REPEATS / 2) << "SSL Avg: " << sll_sum / REPEATS << " Treap Avg: " << treap_sum / REPEATS;
> 237: }
> 238:
These can be removed. We shouldn't have performance benchmarks running on tier1, as they'll use unnecessary CPU and time. We're also removing the treap in favour of the RB-tree soon :-).
-------------
PR Review: https://git.openjdk.org/jdk/pull/20425#pullrequestreview-2601371143
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1946317189
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1946316910
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1946316278
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1946315914
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1946309823
PR Review Comment: https://git.openjdk.org/jdk/pull/20425#discussion_r1946313782
More information about the hotspot-dev
mailing list