8366241: NMT: Consolidate [Virtual/Committed/Reserved]Regions into one structure #27137

afshin-zafari · 2025-09-08T08:55:50Z

The structures CommittedMemoryRegion and ReservedMemoryRegion are merged into the VirtualMemoryRegion.

Tests:
tiers1-5, main platforms, debug/product

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8366241: NMT: Consolidate [Virtual/Committed/Reserved]Regions into one structure (Enhancement - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27137/head:pull/27137
$ git checkout pull/27137

Update a local copy of the PR:
$ git checkout pull/27137
$ git pull https://git.openjdk.org/jdk.git pull/27137/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27137

View PR using the GUI difftool:
$ git pr show -t 27137

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27137.diff

Using Webrev

Link to Webrev Comment

…e structure

bridgekeeper · 2025-09-08T08:56:30Z

👋 Welcome back azafari! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-09-08T08:57:06Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2025-09-08T08:57:39Z

@afshin-zafari The following label will be automatically applied to this pull request:

hotspot-runtime

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

afshin-zafari · 2025-09-08T08:59:48Z

Waiting for 8366363 to be integrated. Those changes to be merged here.

mlbridge · 2025-09-08T09:03:15Z

Webrevs

…idate_structures

openjdk · 2025-09-09T10:08:47Z

@afshin-zafari this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout _8366241_nmt_consolidate_structures
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

…idate_structures

src/hotspot/share/nmt/virtualMemoryTracker.hpp

jdksjolen · 2025-09-29T16:32:32Z

src/hotspot/share/nmt/virtualMemoryTracker.hpp

+
+  bool is_valid() const { return base() != nullptr && size() != 0;}
+
+  inline void  set_reserved_call_stack(const NativeCallStack& stack) { _reserved_stack = stack; }


Is this used? Doesn't it make more sense for these values to be constant and the VMR to be an immutable object (in essence).

No it is not used anymore. Removed.
We cannot make VMR immutable, since we need to return a VMR from tree->find_reseved_region(). We need it to be COPYABLE.

src/hotspot/share/nmt/virtualMemoryTracker.hpp

…idate_structures

bridgekeeper · 2025-10-29T01:05:55Z

@afshin-zafari This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

afshin-zafari · 2025-10-29T11:35:37Z

/keepalive

openjdk · 2025-10-29T11:37:59Z

@afshin-zafari The pull request is being re-evaluated and the inactivity timeout has been reset.

Arraying · 2025-11-20T13:01:45Z

src/hotspot/share/nmt/virtualMemoryTracker.hpp

- private:
-  NativeCallStack  _stack;
-  MemTag         _mem_tag;
+  bool is_valid() const { return base() != nullptr && size() != 0;}


The old structures had the base address and size by default initialised to 1. I think the new "invalid" values of 0 and 0 are more ergonomic, but I'm wondering, what's the reason we used 1 originally?

IIRC, they were used for debugging and tracing some cases that happened during VMATree implementation.

Arraying · 2025-11-20T13:05:52Z

src/hotspot/share/nmt/virtualMemoryTracker.hpp

-   }
+  }
+
+  VirtualMemoryRegion(address addr, size_t size, const NativeCallStack& stack, bool committed) :


Correct me if I'm wrong but is this committed only ever true? And then we use the presence of this constructor parameter to decide whether we are creating a committed or a reserved region?

If so, at the very least we should assert that committed is true. But I think it would be nicer to make the constructor private and make some static factory methods akin to VirtualMemoryRegion::committed and VirtualMemoryRegion::reserved.

You are right. The committed is used here to distinguish signatures of the VMR constructor.
VMR(address, size_t, NativeCallStack) without this parameter would have been resolved to VMR(address, size_t, NCS, MemTag= mtNone). Actually it doesn't matter if the value of the parameter be true or false, the bool type does the work for us.
There is only one instance that uses this signature: regionsTree.inline.hpp:41.
Regarding your suggestion, we can provide a static method to build a Committed memory region and pass into it the required values. Something like:

VMR cmr; VMR::create_cmr(addr, size, stack, cmr);// cmr is passed by ref and filled in

8366241: NMT: Consolidate [Virtual/Committed/Reserved]Regions into on…

cd1ac0c

…e structure

openjdk bot added hotspot-runtime hotspot-runtime-dev@openjdk.org rfr Pull request is ready for review labels Sep 8, 2025

Merge remote-tracking branch 'origin/master' into _8366241_nmt_consol…

1600fee

…idate_structures

afshin-zafari marked this pull request as draft September 9, 2025 07:59

openjdk bot removed the rfr Pull request is ready for review label Sep 9, 2025

after merge with 8366363.

077125f

afshin-zafari marked this pull request as ready for review September 9, 2025 10:07

openjdk bot added merge-conflict Pull request has merge conflict with target branch rfr Pull request is ready for review labels Sep 9, 2025

Merge remote-tracking branch 'origin/master' into _8366241_nmt_consol…

11b20ae

…idate_structures

openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label Sep 9, 2025

afshin-zafari marked this pull request as draft September 9, 2025 10:56

openjdk bot removed the rfr Pull request is ready for review label Sep 9, 2025

afshin-zafari added 2 commits September 9, 2025 14:16

master merge fix

9c32575

Merge remote-tracking branch 'origin/master' into _8366241_nmt_consol…

0016455

…idate_structures

openjdk bot added the merge-conflict Pull request has merge conflict with target branch label Sep 29, 2025

Merge remote-tracking branch 'origin/master' into _8366241_nmt_consol…

ec846aa

…idate_structures

afshin-zafari marked this pull request as ready for review September 29, 2025 09:33

openjdk bot added rfr Pull request is ready for review and removed merge-conflict Pull request has merge conflict with target branch labels Sep 29, 2025

jdksjolen reviewed Sep 29, 2025

View reviewed changes

fixes

1f29d6a

openjdk bot added the merge-conflict Pull request has merge conflict with target branch label Sep 30, 2025

Merge remote-tracking branch 'origin/master' into _8366241_nmt_consol…

944b912

…idate_structures

openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label Sep 30, 2025

Arraying reviewed Nov 20, 2025

View reviewed changes


		bool is_valid() const { return base() != nullptr && size() != 0;}

		inline void set_reserved_call_stack(const NativeCallStack& stack) { _reserved_stack = stack; }

8366241: NMT: Consolidate [Virtual/Committed/Reserved]Regions into one structure #27137

Are you sure you want to change the base?

8366241: NMT: Consolidate [Virtual/Committed/Reserved]Regions into one structure #27137

Conversation

afshin-zafari commented Sep 8, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewing

Uh oh!

bridgekeeper bot commented Sep 8, 2025

Uh oh!

openjdk bot commented Sep 8, 2025

Uh oh!

openjdk bot commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

afshin-zafari commented Sep 8, 2025

Uh oh!

mlbridge bot commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

openjdk bot commented Sep 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jdksjolen Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

afshin-zafari Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bridgekeeper bot commented Oct 29, 2025

Uh oh!

afshin-zafari commented Oct 29, 2025

Uh oh!

openjdk bot commented Oct 29, 2025

Uh oh!

Arraying Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

afshin-zafari Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Arraying Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

afshin-zafari Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

afshin-zafari commented Sep 8, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Sep 8, 2025 •

edited

Loading

mlbridge bot commented Sep 8, 2025 •

edited

Loading