Task #5340
closed
Avoid repeatedly initializing new random generators every time random number is generated
0%
Description
During the refactoring in b8bd5ee965, NFD moved from trying to retrieve a globally shared std::mt19937 object to calling ndn::random::getRandomNumberEngine() instead. This currently yields a new thread local instance of a std::mt19937/RandomNumberEngine object every time it's called. While this is not obviously a functional issue, as we currently generate a new seed sequence every time which should yield a new random number, it is both seemingly inefficient and also makes it more difficult to introduce a seeded RNG for unit testing or trying to otherwise ensure repeatable behavior. It would be sensible to refactor sections of code which rely on this to either use a consistent RandomNumberEngine object local to their instance or to return to maintaining a single common RNG globally.
Updated by Davide Pesavento 9 days ago
Alexander Lane wrote:
This currently yields a new thread local instance of a std::mt19937/RandomNumberEngine object every time it's called.
Not sure what you mean here. If it's thread local it cannot be a new instance every time the function is called. It will only be "new" when the function is called in a "new" thread.
it is both seemingly inefficient
inefficient compared to what? I assume that access to thread-local storage is faster than synchronized access to a single global instance, but I didn't do any benchmarking.
and also makes it more difficult to introduce a seeded RNG for unit testing or trying to otherwise ensure repeatable behavior.
This is true but unrelated to the number of RNG instances that are created. IIRC we have/had some patches for ndnSIM to make the seed configurable or something like that.
use a consistent RandomNumberEngine object local to their instance
Whose instance? either way, this will most likely increase the number of RNG instances in the program, and RNG are potentially large and expensive to create.
or to return to maintaining a single common RNG globally.
Then you'd need to protect it with a mutex or similar, and as I said above I doubt this will be more efficient than thread local storage.
Updated by Alexander Lane 9 days ago
- Status changed from New to Closed
I cannot appear to replicate the issue I had initially observed, which may have been a product of typoes in testing. I will open a more specific change if I can show it to be incorrect behavior.