RFR: 8351661: NMT: VMATree should support separate call-stacks for reserve and commit operations [v5]

[Johan Sjölen]

On Mon, 24 Mar 2025 08:58:54 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

>> In NMT detail mode, we need to have separate call-stacks for Reserve and Commit operations.
>> This PR adds a second stack to every node that will be used when committing (and uncommitting) the start node of a reserved region.
>
> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
> 
>   more fixes.

Some more comments.

Can we have the example cases I presented as test cases in the code? I think that those show the behavior of this code.

test/hotspot/gtest/nmt/test_vmatree.cpp line 764:

> 762:   auto expected =  [&](Tree& tree, int p, SIndex reserve_stack, SIndex commit_stack, int line_no = __LINE__) {
> 763:     VMATree::VMATreap::Range r = tree.tree().find_enclosing_range(p);
> 764:     const bool check_end_node = true;

Dead code

test/hotspot/gtest/nmt/test_vmatree.cpp line 782:

> 780:   };
> 781: 
> 782:   {

I think that each test case should at least have a small comment summarizing the supposed goal of the test. Below is an example of what the could look like for the first test case. 


// Check committing into a reserved region inherits the call stacks

test/hotspot/gtest/nmt/test_vmatree.cpp line 787:

> 785:     expected(tree,  0, si_1,   -1, __LINE__);
> 786: 
> 787:     tree.commit_mapping(25, 25, call_stack_2, true); // commit at the middle of the region

We have something like `[0, 25), [25, 50), [50, 100)` here, can we check the last region also?

test/hotspot/gtest/nmt/test_vmatree.cpp line 807:

> 805: 
> 806:     tree.commit_mapping(10, 20, call_stack_1); // commit with overlap
> 807:     expected(tree, 10, si_1, si_1, __LINE__);

Introduce a third call stack for this case. We want to see that the reserved stack is placed correctly, it also clarifies what the test does a bit for readers.

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

PR Review: https://git.openjdk.org/jdk/pull/24028#pullrequestreview-2713205086
PR Review Comment: https://git.openjdk.org/jdk/pull/24028#discussion_r2011789146
PR Review Comment: https://git.openjdk.org/jdk/pull/24028#discussion_r2011801428
PR Review Comment: https://git.openjdk.org/jdk/pull/24028#discussion_r2011794964
PR Review Comment: https://git.openjdk.org/jdk/pull/24028#discussion_r2011804909