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

Afshin Zafari azafari at openjdk.org
Thu Apr 18 14:27:38 UTC 2024


On Thu, 18 Apr 2024 09:28:05 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   order of params are consistent now. style is corrected.
>
> src/hotspot/os/windows/os_windows.cpp line 5068:
> 
>> 5066: char* os::pd_map_memory(int fd, const char* file_name, size_t file_offset,
>> 5067:                         char *addr, size_t bytes,
>> 5068:                         bool read_only,
> 
> This should be reverted.

Done.

> src/hotspot/share/cds/filemap.cpp line 1701:
> 
>> 1699:                              AlwaysPreTouch ? false : read_only,
>> 1700:                              allow_exec,
>> 1701:                              flags);
> 
> Revert this change

Done.

> src/hotspot/share/cds/filemap.cpp line 1729:
> 
>> 1727:                               false /* !read_only */,
>> 1728:                               r->allow_exec(),
>> 1729:                               mtClassShared);
> 
> This mixes styles between multiple arguments per line vs one argument per line.

related params (file, memory, info) are in the same line.

> src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 217:
> 
>> 215:   if (!_heap_region_special) {
>> 216:     os::commit_memory_or_exit(sh_rs.base(), _initial_size, heap_alignment, !ExecMem,
>> 217:                               "Cannot commit heap memory", mtGC);
> 
> The argument order needs to be changed here and below.

Done.

> src/hotspot/share/runtime/os.hpp line 520:
> 
>> 518:                            bool read_only,
>> 519:                            bool allow_exec,
>> 520:                            MEMFLAGS flag);
> 
> Style inconsistency

related params (file, memory, flags) are in the same line.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18745#discussion_r1570872004
PR Review Comment: https://git.openjdk.org/jdk/pull/18745#discussion_r1570871096
PR Review Comment: https://git.openjdk.org/jdk/pull/18745#discussion_r1570870825
PR Review Comment: https://git.openjdk.org/jdk/pull/18745#discussion_r1570870421
PR Review Comment: https://git.openjdk.org/jdk/pull/18745#discussion_r1570868039


More information about the shenandoah-dev mailing list