RFR: 8366241: NMT: Consolidate [Virtual/Committed/Reserved]Regions into one structure [v6]

[Paul Hübner]

On Tue, 30 Sep 2025 10:09:35 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

>> The structures `CommittedMemoryRegion` and `ReservedMemoryRegion` are merged into the `VirtualMemoryRegion`.
>> 
>> Tests:
>> tiers1-5, main platforms, debug/product
>
> Afshin Zafari has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains nine commits:
> 
>  - Merge remote-tracking branch 'origin/master' into _8366241_nmt_consolidate_structures
>  - fixes
>  - Merge remote-tracking branch 'origin/master' into _8366241_nmt_consolidate_structures
>  - Merge remote-tracking branch 'origin/master' into _8366241_nmt_consolidate_structures
>  - master merge fix
>  - Merge remote-tracking branch 'origin/master' into _8366241_nmt_consolidate_structures
>  - after merge with 8366363.
>  - Merge remote-tracking branch 'origin/master' into _8366241_nmt_consolidate_structures
>  - 8366241: NMT: Consolidate [Virtual/Committed/Reserved]Regions into one structure

src/hotspot/share/nmt/virtualMemoryTracker.hpp line 217:

> 215:   }
> 216: 
> 217:   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`.

src/hotspot/share/nmt/virtualMemoryTracker.hpp line 271:

> 269:   inline const NativeCallStack* committed_call_stack() const         { return &_committed_stack; }
> 270: 
> 271:   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?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27137#discussion_r2546002237
PR Review Comment: https://git.openjdk.org/jdk/pull/27137#discussion_r2545990569