RFR: 8337217: Port VirtualMemoryTracker to use VMATree

[Gerard Ziemski]

On Mon, 26 Aug 2024 07:16:39 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

> > Is the plan to check-in the fix with both paths? Or are we going to remove the linked-list based one after the review?
> 
> The plan is to have both versions available at run-time. In this plan, we will add JVM options to let the user to select either of them. This can be used for: 1️⃣ stepping back if new version fails in some use-cases, and also for 2️⃣ running benchmarks and comparing the results of both.

If we are going to keep the 2 versions for a while, then I would really, really like to see the two implementations as instances of `VirtualMemoryTrackerWithLinkedList` and `VirtualMemoryTrackerWithTree`, and have `VirtualMemoryTracker` be the single class we have the external code deal with, i.e. I think we can do better that:

 ```
 static void snapshot_thread_stacks() {
    if (is_using_sorted_link_list())
      VirtualMemoryTracker::snapshot_thread_stacks();
    if (is_using_tree())
      VirtualMemoryTrackerWithTree::Instance::snapshot_thread_stacks();
  }

> > I still haven't run the benchmarks to verify the speed increase promise. Ideally, I would like to do this with my mechanism, but will only do it, if I can manage it without getting sucked into working on the mechanism itself.
> > I will definitively want to run the provided microbenchmarks though.
> 
> In `test_vmt_with_tree.cpp`, two versions are compared. One of the tests (`PerformanceComparison`) is comparing the time they take for the same operation. This test is skipped for now, because its expectation of time-improvement fails. Until the time we still did not merge all other related changes (like the `copy_flag` or `use_tag_inplace`, [#20330](https://github.com/openjdk/jdk/pull/20330)) to this PR we cannot expect the time to be improved.

Should we say then, that this is blocked by those 2 issues? Is it OK then if I wait till those get checked in before verifying the performance benefits if the new implementation? The performance was the main motivation here, correct?

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

PR Comment: https://git.openjdk.org/jdk/pull/20425#issuecomment-2316167719
PR Comment: https://git.openjdk.org/jdk/pull/20425#issuecomment-2379647798