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

[Gerard Ziemski]

On Wed, 9 Apr 2025 08:12:59 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:
> 
>   fixed format of the missed Pre

Changes requested by gziemski (Reviewer).

src/hotspot/share/nmt/vmatree.cpp line 69:

> 67:     stA.out.set_reserve_stack(NativeCallStackStorage::invalid);
> 68:     stB.in.set_reserve_stack(NativeCallStackStorage::invalid);
> 69:   }

I'd prefer to see this as:


  // set default values, then depending on operation later, fill the required states
  stA.out.set_commit_stack(NativeCallStackStorage::invalid);
  stA.in.set_commit_stack(NativeCallStackStorage::invalid);
  stA.out.set_reserve_stack(NativeCallStackStorage::invalid);
  stB.in.set_reserve_stack(NativeCallStackStorage::invalid);

   if (is_reserve_operation) {
    stA.out.set_reserve_stack(metadata.stack_idx);
    stB.in.set_reserve_stack(metadata.stack_idx);
  }

  if (is_commit_operation) {
    stA.out.set_commit_stack(metadata.stack_idx);
    stB.in.set_commit_stack(metadata.stack_idx);
  }



Than later on we can skip setting these default values altogether and only touch the ones we need to.

src/hotspot/share/nmt/vmatree.cpp line 147:

> 145:         stB.in.set_reserve_stack(leqA_n->val().out.reserved_stack());
> 146:         stA.out.set_commit_stack(NativeCallStackStorage::invalid);
> 147:         stB.in.set_commit_stack(NativeCallStackStorage::invalid);

Wouldn't need to do those if we followed my suggestion above?

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

PR Review: https://git.openjdk.org/jdk/pull/24028#pullrequestreview-2754081239
PR Review Comment: https://git.openjdk.org/jdk/pull/24028#discussion_r2035731645
PR Review Comment: https://git.openjdk.org/jdk/pull/24028#discussion_r2035746412