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

[Johan Sjölen]

On Wed, 17 Apr 2024 06:41:55 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:
>> 
>>   Style, copyright
>
> src/hotspot/share/nmt/nmtTreap.hpp line 187:
> 
>> 185:       free(head);
>> 186:     }
>> 187:     return nullptr;
> 
> ? Why does this return anything

It returns the resulting `TreapNode*`. A `TreapNode*` which is `nullptr` is an empty treap.

> src/hotspot/share/nmt/nmtTreap.hpp line 196:
> 
>> 194:   friend class VMATreeTest;
>> 195:   using CTreap = TreapNode<K, V, CMP>;
>> 196:   CTreap* root;
> 
> What is a CTreap? To my mind, the treap is the tree, and a node in that tree is a treapnode. Can we not just call CTreap a node? And maybe use the same name we used above in TreapNode?
> 
> What meaning has the C?

It's C-Heap allocated :-). Doesn't matter, just wanted a name that connects to the class name and the C sticks out. Changed to Node.

> src/hotspot/share/nmt/nmtTreap.hpp line 197:
> 
>> 195:   using CTreap = TreapNode<K, V, CMP>;
>> 196:   CTreap* root;
>> 197:   uint64_t prng_seed;
> 
> Leading underscore for member vars?

Yes! Thanks.

> src/hotspot/share/nmt/nmtTreap.hpp line 213:
> 
>> 211:     static const uint64_t PrngModMask = (static_cast<uint64_t>(1) << PrngModPower) - 1;
>> 212:     prng_seed = (PrngMult * prng_seed + PrngAdd) & PrngModMask;
>> 213:     return prng_seed;
> 
> Do we need our own random generator here? Why not use os::next_random?

TIL, I wasn't aware of that function. I specifically wanted something where each `Treap` stores its own seed/RNG state.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1568401643
PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1568405212
PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1568402347
PR Review Comment: https://git.openjdk.org/jdk/pull/18289#discussion_r1568403841