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

[Afshin Zafari]

On Tue, 30 Apr 2024 05:31:05 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   alignment in coding style changed.
>
> src/hotspot/share/memory/virtualspace.cpp line 57:
> 
>> 55: }
>> 56: 
>> 57: ReservedSpace::ReservedSpace(size_t size, size_t preferred_page_size, MEMFLAGS flag) : _fd_for_heap(-1), _nmt_flag(flag) {
> 
> Small nit: Mixture of styles. As much as I dislike it, current style is to initialize things via dedicated initialize methods. I'd rather stay consistent.
> 
> That said, I would be more than happy for someone to give these classes a once-over and convert them to the more usual style - using initializer lists. Then, we also can make members const that should be const, e.g. _nmt_flags. Not in this PR though.

Do you mean to move the initializations on line 57 (and others in the files) to the `initialize` method?

> src/hotspot/share/memory/virtualspace.hpp line 46:
> 
>> 44:   int      _fd_for_heap;
>> 45:   bool     _executable;
>> 46:   MEMFLAGS _nmt_flag;
> 
> See my remark below. This member, and probably others (e.g. page size and size) could and should probably be const. Food for follow up PRs.

The getter method made as `const`.

> src/hotspot/share/memory/virtualspace.hpp line 71:
> 
>> 69:  public:
>> 70: 
>> 71:   MEMFLAGS nmt_flag() { return _nmt_flag; }
> 
> const method

Done.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18745#discussion_r1584392835
PR Review Comment: https://git.openjdk.org/jdk/pull/18745#discussion_r1584393529
PR Review Comment: https://git.openjdk.org/jdk/pull/18745#discussion_r1584389450