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

[Johan Sjölen]

On Fri, 22 Mar 2024 13:34:34 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:
>> 
>>   Include os.inline.hpp
>
> src/hotspot/share/nmt/vmatree.hpp line 63:
> 
>> 61:     }
>> 62:   };
>> 63: 
> 
> In my original proposal, a `MappingState` (https://github.com/tstuefe/jdk/blob/6be830cd2e90a009effb016fbda2e92e1fca8247/src/hotspot/share/nmt/vmaTree.cpp#L106C7-L114) is the combination of ( MEMFLAGS + VMAstate ).
> 
> A `MappingStateChange` https://github.com/tstuefe/jdk/blob/6be830cd2e90a009effb016fbda2e92e1fca8247/src/hotspot/share/nmt/vmaTree.cpp#L38-L65 is the combination of two `MappingState`. Its "is_noop()" method compares the combination of both ( MEMFLAGS + VMAstate ) tuples.
> 
> This `State` here is neither. It contains only the incoming VMAState, and carries the full information only for the outgoing state. Its ` is_noop` only compares VMAState. This renders `is_noop()` pretty useless since it alone is not sufficient when checking if areas for mergeability. Consequently, you then tree-search the adjacent node whenever is_noop is used ( e.g. line 161, `if (is_noop(stA) && Metadata::equals(stA.metadata, leqA_n->val().metadata)) { ` .
> 
> I would suggest following my proposal here and carrying the full information for both incoming and outgoing states. Advantages would be:
> - it removes the need for tree searches whenever we check whether neighboring areas can be folded.
> - it would also make the code easier to read and arguably more robust
> - it is arguably more logical - semantically, there is no difference between VMAState and the rest of the state information, all of them are part of the total region state that determines if regions are mergeable.
> 
> If you are worried about data size: all the information we need can still be encoded in a 64-bit value, as suggested above. And compared with a single cmpq. 16 bits for MEMFLAGS, 2 bits for VMAState, and 32 bits are more than enough as a callstack id (in fact, 16 bits should be enough).

Hi, just to be clear: We don't perform a tree search whenever `is_noop` is used, the `leqA_n` is also present in your draft. Still, I think you might be right that having the full state for both in and out is the best way to go, I'll try it out as a simplification.

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

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