RFR: 8330076: [NMT] add/make a mandatory MEMFLAGS argument to family of os::reserve/commit/uncommit memory API [v3]

[Johan Sjölen]

On Thu, 11 Apr 2024 16:21:52 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

>> `MEMFLAGS flag` is used to hold/show the type of the memory regions in NMT. Each call of NMT API requires a search through the list of memory regions.
>> The Hotspot code reserves/commits/uncommits memory regions and later calls explicitly NMT API with a specific memory type (e.g., `mtGC`, `mtJavaHeap`) for that region.  Therefore, there are two search in the list of regions per reserve/commit/uncommit operations, one for the operation and another for setting the type of the region.  
>> When the memory type is passed in during reserve/commit/uncommit operations, NMT can use it and avoid the extra search for setting the memory type.
>> 
>> Tests: tiers1-5 passed on linux-x64, macosx-aarch64 and windows-x64 for debug and non-debug builds.
>
> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fixed shenandoah missed changes.

Hi Afshin,

Thank you for this!

I found a couple of things.

@tstuefe , would you mind having a look at the Metaspace changes?

src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 2267:

> 2265:   char* start = (char*) _bitmap_region.start() + off;
> 2266: 
> 2267:   if (!os::commit_memory(start, len, false)) {

I think this should probably be `mtGC`.  @shipilev, you don't happen to know whether this should be accounted under the Java heap? Thank you.

src/hotspot/share/nmt/virtualMemoryTracker.cpp line 460:

> 458:   assert(_reserved_regions != nullptr, "Sanity check");
> 459: 
> 460:   ReservedMemoryRegion  rgn(addr, size, NativeCallStack::empty_stack(), flag);

Instead, change the constructor so that it takes a flag?

```c++
  ReservedMemoryRegion(address base, size_t size, MEMFLAGS flag) :
    VirtualMemoryRegion(base, size), _stack(NativeCallStack::empty_stack()), _flag(flag) { }


Or does that break somewhere else?

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

PR Review: https://git.openjdk.org/jdk/pull/18745#pullrequestreview-1994902004
PR Review Comment: https://git.openjdk.org/jdk/pull/18745#discussion_r1561430517
PR Review Comment: https://git.openjdk.org/jdk/pull/18745#discussion_r1561443098