RFR: 8335091: NMT: VMATree reserve_mapping and commit_mapping APIs need MEMFLAGS while un/-committing API has no MEMFLAGS arg [v8]

Johan Sjölen jsjolen at openjdk.org
Wed Sep 25 06:58:41 UTC 2024


On Tue, 24 Sep 2024 17:17:04 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   logging/log.hpp is included
>
> src/hotspot/share/nmt/vmatree.hpp line 184:
> 
>> 182:   SummaryDiff commit_mapping(position from, position sz, const RegionData& metadata, bool copy_flag = false) {
>> 183:     return register_mapping(from, from + sz, StateType::Committed, metadata, copy_flag);
>> 184:   }
> 
> Why do we need this API? It's only used in the test so far, so do we really need it?
> 
> I am not too crazy about `register_mapping` basically being a wrapper around `register_mapping(.., true)`
> 
> What if someone calls `commit_mapping(.., true)` ? Doesn't that turn into `register_mapping(..., StateType::Committed, metadata, true)` which is the same as `uncommit_mapping()` ? It doesn't make sense?

I think it makes sense to have these short definitions that just expands to the right incantation to `register_mapping`, otherwise it would just have to happen at the place where the caller uses `VMATree`. We will need all of the APIs very soon, with the integration of Afshin's port of VirtualMemoryTracker to VMATree.

>What if someone calls commit_mapping(.., true) ? Doesn't that turn into register_mapping(..., StateType::Committed, metadata, true) which is the same as uncommit_mapping() ? It doesn't make sense?

That's true, it shoudn't be `StateType::Committed` in `uncommit_mapping`, it should be reserved!!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20330#discussion_r1774607547


More information about the hotspot-runtime-dev mailing list