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