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

[Afshin Zafari]

On Fri, 12 Apr 2024 07:33:51 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   review comments applied.
>
> src/hotspot/share/memory/virtualspace.cpp line 615:
> 
>> 613: 
>> 614: ReservedHeapSpace::ReservedHeapSpace(size_t size, size_t alignment, size_t page_size, const char* heap_allocation_directory) : ReservedSpace() {
>> 615:   set_nmt_flag(mtJavaHeap);
> 
> It seems odd that we only initialize the _nmt_flag when `size == 0`. Could this be done after that check? If not, why not?
> 
> There's also a call to record_virtual_memory_type further down in the code. Why is that needed? Why isn't it enough to pass in the correct type to the os::reserve_memory call in the initialize function?

Corrected.

> src/hotspot/share/memory/virtualspace.cpp line 672:
> 
>> 670:                                      size_t rs_align,
>> 671:                                      size_t rs_page_size) : ReservedSpace() {
>> 672:   set_nmt_flag(mtCode);
> 
> Why isn't this a part of the initialize call? This looks like a bug to me. `initialize` will call clear_members, which will undo this setting.

`set_nmt_flag()`, `initialize` and `initialize_members` are chagned accorrding to the related comments.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18745#discussion_r1566006111
PR Review Comment: https://git.openjdk.org/jdk/pull/18745#discussion_r1566009377