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