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

Johan Sjölen jsjolen at openjdk.org
Mon Apr 22 12:55:36 UTC 2024


On Fri, 19 Apr 2024 09:49:33 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:
> 
>   removed extra blank line.

A couple of questions, looks good over all.

src/hotspot/os/windows/os_windows.cpp line 5108:

> 5106: 
> 5107:     base = (char*) virtualAlloc(addr, bytes, MEM_COMMIT | MEM_RESERVE,
> 5108:                                 PAGE_READWRITE);

Why is this removed?

src/hotspot/share/memory/virtualspace.cpp line 45:

> 43: // Dummy constructor
> 44: ReservedSpace::ReservedSpace() : _base(nullptr), _size(0), _noaccess_prefix(0),
> 45:     _alignment(0), _fd_for_heap(-1), _special(false), _executable(false), _nmt_flag(mtNone) {

Isn't just `_flag` or `_memflag` sufficient as a name for `ReservedSpace`? We don' use `nmt_flag` anywhere else in the codebase.

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

PR Review: https://git.openjdk.org/jdk/pull/18745#pullrequestreview-2014652979
PR Review Comment: https://git.openjdk.org/jdk/pull/18745#discussion_r1574692818
PR Review Comment: https://git.openjdk.org/jdk/pull/18745#discussion_r1574703805


More information about the shenandoah-dev mailing list