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

Afshin Zafari azafari at openjdk.org
Mon May 27 21:28:01 UTC 2024


On Mon, 27 May 2024 13:04:32 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> If your comment refers only to these lines of code, they are already verified. Since, inside the split function, the sub-regions get the new flags and all the reserved and committed amounts are moved from the large region to the new ones. So, the accounting of memory is correct.
>> 
>> FWIW, if we trace down the call at line 1346 of `total_space_rs = Metaspace::reserve_address_space_for_compressed_classes(total_range_size, false /* optimize_for_zero_base */);` the region may get different flags of `mtClass` or `mtMetaspace` based on the checked criteria down there.
>>  
>> If you comment on all such cases, then I will double check for them and add assertion for.
>
> My point is that the `archive_space_rs` and `class_space_rs` can get the wrong flags assigned to them. The split functions don't change them. Right?
> 
> I would like to see the code run through our testing with these checks:
> 
>   assert(archive_space_rs.nmt_flag() == mtClassShared, "Sanity");
>   assert(class_space_rs.nmt_flag() == mtClass, "Sanity");

The call to `MemTracker::record_virtual_memory_split_reserved` at line 1364, takes two flags for the split parts. The corresponding regions in NMT take that flags.
The sanity assertions will be added anyway.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19343#discussion_r1616398251


More information about the shenandoah-dev mailing list