On Mon, 26 Aug 2024 07:16:39 GMT, Afshin Zafari <azaf...@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
