On Thu, 1 Aug 2024 15:44:32 GMT, Afshin Zafari <azaf...@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. Thanks for your comments. They are replied or applied. More fixes and questions. Review comments applied. The comments are applied. Performance test runs insert/remove operations for SortedLinkList (SLL in the code) and Treap and expects that Treap be faster. Both tests pass with a factor of 70+ faster for 10000 elements. Dear reviewers in recent commit the following can be considered: ### Changes made to the code - Instance/Static implemented. - Using of Reference instead of pointers, as much as possible. - Removed extra un-needed args from functions - `NodeHelper` is implemented using composition approach. - Non-used methods are removed from `NodeHelper` ### Still open conversations from previous reviews - Two alternatives for implementing `apply_summary_diff` - Is readability improved after latest changes? - Are `lambda` usages acceptable now? Any improvement suggestion? - Which pointers still remained to be replaced by references? GHA failures are for some tests that have already a related JBS issue, not for this PR. > ```c++ > region->add_committed_region(committed_start, committed_size, ncs); // <-- > LOST! > ``` The `region` is not a VMATree::node, it is a `ReservedMemoryRegion*`. Dear @tstuefe, this PR has been reviewed couple of rounds. Would you please, give your feedback? Thanks. ------------- PR Review: https://git.openjdk.org/jdk/pull/20425#pullrequestreview-2217791061 PR Review: https://git.openjdk.org/jdk/pull/20425#pullrequestreview-2219547262 PR Review: https://git.openjdk.org/jdk/pull/20425#pullrequestreview-2333278135 PR Review: https://git.openjdk.org/jdk/pull/20425#pullrequestreview-2365877947 PR Comment: https://git.openjdk.org/jdk/pull/20425#issuecomment-2273208292 PR Comment: https://git.openjdk.org/jdk/pull/20425#issuecomment-2283953888 PR Comment: https://git.openjdk.org/jdk/pull/20425#issuecomment-2380017488 PR Comment: https://git.openjdk.org/jdk/pull/20425#issuecomment-2410734124 PR Comment: https://git.openjdk.org/jdk/pull/20425#issuecomment-2462154991
