RFR: 8255254: Split os::reserve_memory and os::map_memory_to_file interfaces

[Anton Kozlov]

On Mon, 26 Oct 2020 08:23:12 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> Hi,
>> 
>> Please review a change to extract map_memory_to_file interface out of reserve_memory when the latter takes file descriptor.
>> 
>> The change should be a pure refactoring without changes in functionality. The only part is disturbing: a comment in original os_posix.cpp:316 seems to refer to else clause and it contradicts to the actual code.
>
> src/hotspot/os/posix/os_posix.cpp line 362:
> 
>> 360:   // After we have an aligned address, we can replace anonymous mapping with file mapping
>> 361:   if (replace_existing_mapping_with_file_mapping(aligned_base, size, file_desc) == NULL) {
>> 362:     vm_exit_during_initialization(err_msg("Error in mapping Java heap at the given filesystem directory"));
> 
> There shouldn't be a need to use err_msg for plain strings.

Agree, I haven't paid attention while I was moving lines. Unfortunately, there are more cases for exactly this line https://github.com/openjdk/jdk/search?q=%22Error+in+mapping+Java+heap%22
I think it would interesting to find and eliminate all of them at once. I've filed https://bugs.openjdk.java.net/browse/JDK-8255416

> src/hotspot/share/runtime/os.cpp line 1742:
> 
>> 1740:   }
>> 1741:   return result;
>> 1742: }
> 
> It's a bit unfortunate that the two functions behave differently w.r.t. NMT. They have the same name, but only one reports to NMT. I'd personally would like to transition the code base so that all os::<operation>_memory functions report to NMT, and if you don't wan that you'd use pd_ or internal functions. I think there's a pre-existing bug where a call to os::map_memory_to_file misses the NMT reporting. This is a recurring problem in the os:: layer. Since this is already problematic before your changes, we can investigate this as a separate JBS bug.

Nice catch. Agree, this would be out of scope of this patch. I've filed https://bugs.openjdk.java.net/browse/JDK-8255414

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

PR: https://git.openjdk.java.net/jdk/pull/812