RFR: 8312132: Add tracking of multiple address spaces in NMT [v74]

[Johan Sjölen]

On Fri, 10 May 2024 11:38:36 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Johan Sjölen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Some style
>
> src/hotspot/share/nmt/vmatree.hpp line 67:
> 
>> 65: 
>> 66:     Metadata(NativeCallStackStorage::StackIndex stack_idx, MEMFLAGS flag)
>> 67:     : stack_idx(stack_idx), flag(flag) {}
> 
> I would assert here that with state=released, we only ever want to see mtNone.

We don't have the state here, do you mean in `IntervalState` ctr? I'm doing `!(type == Released) || flag == mtNone` instead of `!=` to make the usage of `~P v Q equiv. P => Q` obvious.

> src/hotspot/share/nmt/vmatree.hpp line 91:
> 
>> 89:     StateType type() const {
>> 90:       return static_cast<StateType>(type_flag[0]);
>> 91:     }
> 
> Proposal: provide `is_reserved()` and `is_committed()` and replace manual comparisons with the state enum with those. Easier on the eye.

Afshin mentioned this too, I believe, but I want to push back here. I prefer showing what we're doing here (simple comparison) rather than hiding it behind a utility function. Requires less jumping around when reading unknown code.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1598375334
PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1598378197