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

[Johan Sjölen]

On Mon, 29 Jul 2024 12:43:38 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> src/hotspot/share/nmt/vmatree.cpp line 37:
>> 
>>> 35: 
>>> 36: VMATree::SummaryDiff VMATree::register_mapping(position A, position B, StateType state,
>>> 37:                                                const RegionData& metadata, bool copy_flag) {
>> 
>> We need to think about what the behavior should be if the caller passes `copy_flag=true` but vmatree disagrees and thinks the caller should specify a flag. At the moment we quietly take over the flag from the `metadata` argument, but I think we should assert. Something like "reserving without pre-existing range requires a valid NMT flag".
>> 
>> Tied into that is the question of what the flag in the `metadata` argument should be for `copy_flag=true`. I would argue that the flag must be `mtNone` - any other flag would contradict the "copy_flag" argument.
>> 
>> In fact, we could argue that `mtNone` means `copy_flag==true` and then we don't need the new `copy_flag` argument. In that case, reserving with `mtNone` means "no clue about the flag, you figure it out" and then it requires a pre-existing mapping. Are there valid use cases for reserving ranges with mtNone as a valid NMT flag?
>
> Asserting may make sense on debug builds, as you might be making a mistake, but we should have pre-defined behavior for production builds anyhow, no `ShouldNotReachHere()`. I think that valid behavior is to set the flag to whichever was provided in the call.
> 
> 
> Another option is to have a more elaborate heuristic. If you uncommit `[a, b)` then find the first range which starts inside of `[a, b)` and take its memory flag, if none is found then give up and take `mtNone`.
> 
> The nicest behavior is an `O(n)` algorithm where we go through all regions in `[a, b)` and convert each to its reserved version with the flag, then uncommit with holes in the range would leave those holes unreserved. 
> 
>>Are there valid use cases for reserving ranges with mtNone as a valid NMT flag?
> 
> Only in the context of then later setting the memory flag, which I think is a pattern which should be avoided if possible.

I'm only considering the usage of this flag as something which should be used by an `uncommit_region` API, if that wasn't clear.

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

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