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

[Thomas Stuefe]

On Tue, 30 Apr 2024 09:54:31 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>>> Sure, the allocator isn't responsible for seeding the nodes. It was simply convenient that the class which does allocation also happens to hold the seed, so I could have them do both together.
>>> 
>>> I came to the opposite conclusion that you did: I didn't want to have a global shared state seed as that makes me have to think about the behavior of the RNG in the presence of multiple threads and instances of the treap.
>>> 
>> 
>> Yeah sure, but now you get collisions if you merge two trees together that both have had nodes added before the merge. Because both Treaps will have followed the same rng sequence - after all, AFAICS your seed is always the same. 
>> 
>> Either use a global seed, or initialize each Treap seed randomly. The former needs cas, the latter needs an os::random call on each constructor. Pick your poison :) I also think you do not need the seed constructor argument if you do that.
>
>> Yeah sure, but now you get collisions if you merge two trees together that both have had nodes added before the merge. Because both Treaps will have followed the same rng sequence - after all, AFAICS your seed is always the same.
> 
> Alright, yeah, that's a problem if we make that a supported API. `os::random` for each `Treap` constructor and then saving that as the initial seed seems reasonable to me and like a clear improvement.
> 
>>I also think you do not need the seed constructor argument if you do that.
> 
> That's true, it might be nice to have a seed constructor argument if you want reproducible tests for example.

Cool for me. Feel free to close this conversation :)

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

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