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

Afshin Zafari azafari at openjdk.org
Mon Apr 15 15:41:06 UTC 2024


On Fri, 12 Apr 2024 07:45:43 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/nmt/virtualMemoryTracker.hpp line 307:
> 
>> 305: 
>> 306:   ReservedMemoryRegion(address base, size_t size, MEMFLAGS flag) :
>> 307:     VirtualMemoryRegion(base, size), _stack(NativeCallStack::empty_stack()), _flag(flag) { }
> 
> The function above uses mtNone. I find that a bit dubious, but I understand that it is done to be able to write code like this:
> 
>     ReservedMemoryRegion* rmr = VirtualMemoryTracker::_reserved_regions->find(ReservedMemoryRegion(addr, size));
> 
> 
> Unfortunately, it opens up the door for people to accidentally use that version instead of this new version that you have written. Could we get rid of the version using mtNone somehow?
> 
> The same question goes for the version above that, which has a "MEMFLAGS flag = mtNone". (GH doesn't allow me to comment on lines that you haven't changed)

`mtNone` as default value is no longer valid.

> src/hotspot/share/runtime/os.hpp line 511:
> 
>> 509:   // and is added to be used for implementation of -XX:AllocateHeapAt
>> 510:   static char* map_memory_to_file(size_t size, int fd, MEMFLAGS flag = mtNone);
>> 511:   static char* map_memory_to_file_aligned(size_t size, size_t alignment, int fd, MEMFLAGS flag);
> 
> There are still a few mtNone usages in this file.

`mtNone` as default value for optional arguments is removed from all function definitions.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18745#discussion_r1565995694
PR Review Comment: https://git.openjdk.org/jdk/pull/18745#discussion_r1565997114


More information about the shenandoah-dev mailing list