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

[Gerard Ziemski]

On Mon, 12 May 2025 11:21:41 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 27 additional commits since the last revision:
> 
>  - Merge remote-tracking branch 'origin/master' into _8351661_separate_call_stack_reserve_commit
>  - fixed one case.
>  - test with random requests passes now.
>  - semantic text added.
>  - more tests for full coverage of update_region.
>  - fixes for cross-compilations
>  - removed print_on for release build success.
>  - complete code coverage for new impl. All tests pass.
>  - new version. All tests except one passed.
>  - second try
>  - ... and 17 more: https://git.openjdk.org/jdk/compare/66c958b0...043c11d6

I reviewed a section of test_vmatree.cpp using my code to output the visual representation of the tree. So instead of:


TEST_VM_F(NMTVMATreeTest, SummaryAccounting) {
  { // Fully enclosed re-reserving works correctly.
    Tree::RegionData rd(NCS::StackIndex(), mtTest);
    Tree::RegionData rd2(NCS::StackIndex(), mtNMT);
    Tree tree;
    VMATree::SummaryDiff all_diff = tree.reserve_mapping(0, 100, rd);
    VMATree::SingleDiff diff = all_diff.tag[NMTUtil::tag_to_index(mtTest)];
    EXPECT_EQ(100, diff.reserve);
    all_diff = tree.reserve_mapping(50, 25, rd2);
    diff = all_diff.tag[NMTUtil::tag_to_index(mtTest)];
    VMATree::SingleDiff diff2 = all_diff.tag[NMTUtil::tag_to_index(mtNMT)];
    EXPECT_EQ(-25, diff.reserve);
    EXPECT_EQ(25, diff2.reserve);
  }


I have:


TEST_VM_F(NMTVMATreeTest, SummaryAccounting) {
  { // Fully enclosed re-reserving works correctly.
    Tree::RegionData rd(NCS::StackIndex(), mtTest);
    Tree::RegionData rd2(NCS::StackIndex(), mtNMT);
    Tree tree;
    VMATree::SummaryDiff all_diff = tree.reserve_mapping(0, 100, rd);
//            1         2         3         4         5         6         7         8         9         10         11
//  01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789
//  AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA..........
//  Legend:
//  A - Test (reserved)
//  . - free
    VMATree::SingleDiff diff = all_diff.tag[NMTUtil::tag_to_index(mtTest)];
    EXPECT_EQ(100, diff.reserve);
    all_diff = tree.reserve_mapping(50, 25, rd2);
//              1         2         3         4         5         6         7         8         9         10         11
//    01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789
//    AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABBBBBBBBBBBBBBBBBBBBBBBBBCCCCCCCCCCCCCCCCCCCCCCCCC..........
//    Legend:
//     A - Test (reserved)
//     B - Native Memory Tracking (reserved)
//     C - Test (reserved)
//     . - free
    diff = all_diff.tag[NMTUtil::tag_to_index(mtTest)];
    VMATree::SingleDiff diff2 = all_diff.tag[NMTUtil::tag_to_index(mtNMT)];
    EXPECT_EQ(-25, diff.reserve);
    EXPECT_EQ(25, diff2.reserve);
  }



To help me with the review. I will be doing this to all the cases eventually, it just helps me visualize what's going on...

Here is a diff file, in case you would like to apply such comments to all of the TEST_VM_F(NMTVMATreeTest, SummaryAccounting):

[gerards_patch.diff.zip](https://github.com/user-attachments/files/20213445/gerards_patch.diff.zip)

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

PR Comment: https://git.openjdk.org/jdk/pull/24028#issuecomment-2881356093