47

My program needs to generate many random integers in some range (int min, int max). Each call will have a different range. What is a good (preferably thread-safe) way to do this? The following is not thread-safe (and uses rand(), which people seem to discourage):

int intRand(const int & min, const int & max)
{
    return (rand() % (max+1-min)) + min;
}

This is much slower, but uses <random>:

int intRand(const int & min, const int & max) {
    std::default_random_engine generator;
    std::uniform_int_distribution<int> distribution(min,max);
    return distribution(generator);
}

Something like this is what I'm going for (the changeParameters function doesn't exist though):

int intRand(const int & min, const int & max) {
    static std::default_random_engine generator;
    static std::uniform_int_distribution<int> distribution(0, 10);
    distribution.changeParameters(min, max);
    return distribution(generator);
}

Another option would be to make a wide range on the uniform_int_distribution and then use mod like in the first example. However, I'm doing statistical work, so I want the numbers to come from as unbiased of a distribution as possible (e.g., if the range of the distribution used is not a multiple of (max-min), the distribution will be slightly biased). This is an option, but again, I would like to avoid it.

SOLUTION This solution comes from the answers by @konrad-rudolph @mark-ransom and @mathk . The seeding of the random number generator is done to suit my particular needs. A more common approach would be to use time(NULL). If you make many threads in the same second, they would then get the same seed though. Even with clock() this is an issue, so we include the thread id. A drawback - this leaks memory --- one generator per thread.

#if defined (_MSC_VER)  // Visual studio
    #define thread_local __declspec( thread )
#elif defined (__GCC__) // GCC
    #define thread_local __thread
#endif

#include <random>
#include <time.h>
#include <thread>

using namespace std;

/* Thread-safe function that returns a random number between min and max (inclusive).
This function takes ~142% the time that calling rand() would take. For this extra
cost you get a better uniform distribution and thread-safety. */
int intRand(const int & min, const int & max) {
    static thread_local mt19937* generator = nullptr;
    if (!generator) generator = new mt19937(clock() + this_thread::get_id().hash());
    uniform_int_distribution<int> distribution(min, max);
    return distribution(*generator);
}
PThomasCS
  • 1,307
  • 3
  • 11
  • 17
  • 1
    using % (modulus) gives you the less random lower bits. divide instead. – Mitch Wheat Jan 20 '14 at 15:36
  • 2
    `rand() % (max+1-min)` is most of the time note unifmormly distrubuted. – mathk Jan 20 '14 at 15:38
  • 2
    Don’t use `default_random_engine` – since it’s implementation-defined, it basically offers *no* practical guarantees. You should usually just use `mt19937` instead. – Konrad Rudolph Jan 20 '14 at 15:42
  • The second function is probably slow just because you are recreating the generator on each call (which you are avoiding in the third function using static). Any chance that you could create the generator just once? And change it just when necessary? – Pedrom Jan 20 '14 at 15:42
  • @ Mitch Wheat - can you explain more? @ Davidbrcz I have a Markov decision process with a large finite number of states, each of which has a different number of possible actions. When using epsilon-greedy Sarsa(lambda), I need to randomly sample (uniformly) from the number of available actions, which changes with each state. td;dr: Beacuse I need to. @ mathk - What do you propose instead? Why is it not uniformly distributed - just becaue RAND_MAX is not a multiple or our range? – PThomasCS Jan 20 '14 at 15:45
  • For maximum efficiency do not share generators between threads. See this question: http://stackoverflow.com/q/8285067/395718 – Dialecticus Jan 20 '14 at 16:07
  • There should be no need for the `new` (and the associated memory leak). `thread_local mt19937 generator(seed)` should work, and might make the function slightly faster since you won't be checking and dereferencing a pointer. – Mike Seymour Jan 21 '14 at 11:34
  • @mike-seymour It sounds like *in theory* it would work without the new. In practice (using Visual Studio 2013 Pro or gcc 4.7.2 [IIRC]) thread_local doesn't support complex types. So, the code I posted above can be used now. Perhaps some time in the future, the code posted below by Konrad (which is what you suggest) will actually compile using common compilers. – PThomasCS Jan 22 '14 at 14:47
  • 4
    Won't you get around the memory leak by using a `std::unique_ptr`? – jco Jan 26 '15 at 09:22
  • The solution implementation is not thread safe, since the test of 'generator' is non-atomic with its assignment. Also, it is not actually a memory leak to assign storage per thread. – evoskuil Oct 21 '16 at 20:47
  • My mistake, the use of thread_local makes the assignment thread safe, and the new allocation will of course leak... derp. – evoskuil Oct 21 '16 at 22:18
  • You don't need to generate the seed on your own, C++11 standard can handle it automatically. First `random_device rd;` will obtain a seed from the hardware random number engine or emulate one using software. Then `mt19937_64 gen(rd());` will create a Mersenne Twister random number engine with that seed. – user5280911 Mar 23 '17 at 22:12
  • One more tip: On Linux, put the C++11 standard code of random number generation in C++11 standard multi-thread code, don't put it in pthread code. I encountered program stall in the latter situation which causes my headache for a couple of days. – user5280911 Mar 23 '17 at 22:18

5 Answers5

50

Have you tried this?

int intRand(const int & min, const int & max) {
    static thread_local std::mt19937 generator;
    std::uniform_int_distribution<int> distribution(min,max);
    return distribution(generator);
}

Distributions are extremely cheap (they will be completely inlined by the optimiser so that the only remaining overhead is the actual random number rescaling). Don’t be afraid to regenerate them as often as you need – in fact, resetting them would conceptually be no cheaper (which is why that operation doesn’t exist).

The actual random number generator, on the other hand, is a heavy-weight object carrying a lot of state and requiring quite some time to be constructed, so that should only be initialised once per thread (or even across threads, but then you’d need to synchronise access which is more costly in the long run).

Konrad Rudolph
  • 530,221
  • 131
  • 937
  • 1,214
  • This is the best so far. However, it is slower than the first (based on a couple small experiments). The first method takes 1.272 seconds to make 100000000 random numbers in different ranges on my machine, and your method takes 1.655 seconds (these are averaged over a few trials). On the up-side, your method is likely less biased. – PThomasCS Jan 20 '14 at 15:51
  • 2
    @PThomasCS Maybe so, but it yields correct results. The first doesn’t. the way you use `rand`, besides not being thread-safe, is severely biased. MT19973 is a much higher quality random number generator (and for a Markov decision process you might need this quality). The only correct way of using `rand` (but its quality would still be inferior) is via sampling from a distribution whose size is a whole multiple of your actual distribution’s size, in a loop. That’s also less efficinet. – Konrad Rudolph Jan 20 '14 at 15:57
  • @KonradRudolph You don't need synchronisation if you have one generator per thread using TLS. And one per thread is still cheap. – mathk Jan 20 '14 at 16:02
  • @mathk What synchronisation? That was the *alternative* to using TLS. – Konrad Rudolph Jan 20 '14 at 16:02
  • This code does not compile. In visual studio 2013, using __declspec (thread), I get the error 1>main.cpp(10): error C2483: 'generator' : object with constructor or destructor cannot be declared 'thread' Using g++ and __thread I get: error: `generator' cannot be thread-local becuase ith as non-trivial type `std::mt19937 ... – PThomasCS Jan 20 '14 at 16:08
  • 6
    For this approach you should definitely seed the engine (eg. using `std::random_device`) to avoid different threads coming up with the same sequence of numbers. – ComicSansMS Jan 20 '14 at 16:09
  • @PThomasCS Ah that’s unfortunate: apparently [your compilers are outdated](http://stackoverflow.com/a/12049410/1968). You might have to implement TLS manually then. – Konrad Rudolph Jan 20 '14 at 16:15
  • @PThomasCS use the native TLS implementation with TlsAlloc/TlsGetValue/TlsSetValue ... – mathk Jan 20 '14 at 16:16
  • @KonradRudolph It is not due to gcc the OP is using VisualStudio. – mathk Jan 20 '14 at 16:17
  • @PThomasCS Also note that [boost has a TLS implementation](http://www.boost.org/doc/libs/1_55_0/doc/html/thread/thread_local_storage.html) as part of Boost.Thread that works with Visual C++ (at least VC9 and higher). Unfortunately this one requires a heap allocation. – ComicSansMS Jan 20 '14 at 16:17
  • @mathk According to the comment, OP tried it on both, hence my use of the plural “your compliers”. – Konrad Rudolph Jan 20 '14 at 16:21
  • @PThomasCS if you can't make the generator thread-local how about just a *pointer* to a heap-allocated generator? – Mark Ransom Jan 20 '14 at 17:02
  • @mark-ransom That worked. This together with what @ konrad-rudolph and @ mathk have suggested seems to be a good solution. Later today I'll update my post with the final code (made to compile using gcc or visual studio). – PThomasCS Jan 20 '14 at 17:42
  • I think the static keyword is not necessary since it is implied by thread_local keyword, is it? – ggg Dec 12 '14 at 08:52
  • 1
    @ggg Correct, it’s redundant (though legal). – Konrad Rudolph Dec 12 '14 at 11:52
5

Make the generator static, so it's only created once. This is more efficient, since good generators typically have a large internal state; more importantly, it means you are actually getting the pseudo-random sequence it generates, not the (much less random) initial values of separate sequences.

Create a new distribution each time; these are typically lightweight objects with little state, especially one as simple as uniform_int_distribution.

For thread safety, options are to make the generator thread_local, with a different seed for each thread, or to guard it with a mutex. The former is likely to be faster, especially if there's a lot of contention, but will consume more memory.

Mike Seymour
  • 249,747
  • 28
  • 448
  • 644
  • 2
    Are you sure that a static generator is thread safe? One per thread would be a better choice. Using TLS. – mathk Jan 20 '14 at 16:00
  • @mathk: I'm sure that it isn't; hence the last paragraph describing two options to make it thread safe. – Mike Seymour Jan 20 '14 at 16:01
  • Ah yeap I see, but `thread_local` is not always portable. I suggest to use the implementation from the OS in used. – mathk Jan 20 '14 at 16:07
  • 4
    @mathk: It's as portable as any other standard language feature. You might need to mess around with alternatives if you're using a compiler that doesn't support it; but that will be less portable, so I'd suggest using the standard feature if possible. – Mike Seymour Jan 20 '14 at 16:10
0

You can use one default_random_engine per thread using Thread Local Storage.

I can not tell you how to correctly use TLS since it is OS dependent. The best source you can use is to search through the internet.

mathk
  • 7,973
  • 6
  • 45
  • 74
0

I am a person from the future with the same problem. The accepted answer won't compile on MSVC 2013, because it doesn't implement thread_local (and using __declspec(thread) doesn't work because it doesn't like constructors).

The memory leak in your solution can be moved off the heap by modifying everything to use placement new.

Here's my solution (combined from a header and source file):

#ifndef BUILD_COMPILER_MSVC
thread_local std::mt19937 _generator;
#else
__declspec(thread) char _generator_backing[sizeof(std::mt19937)];
__declspec(thread) std::mt19937* _generator;
#endif
template <typename type_float> inline type_float get_uniform(void) {
    std::uniform_real_distribution<type_float> distribution;
    #ifdef BUILD_COMPILER_MSVC
        static __declspec(thread) bool inited = false;
        if (!inited) {
            _generator = new(_generator_backing) std::mt19937();
            inited = true;
        }
        return distribution(*_generator);
    #else
        return distribution(_generator);
    #endif
}
geometrian
  • 14,775
  • 10
  • 56
  • 132
  • In the MSVC version, how would you release the `_generator`s in each thread? – Zhao Xiang Nov 06 '15 at 12:18
  • @ZhaoXiang Unlike `new`, placement-`new` allocates wherever you told it to--here, in BSS, not the heap--so you don't deallocate it. The destructor is trivial, so you don't need to call that either. – geometrian Nov 06 '15 at 16:47
-2

Write a simple LCG (or whatever) PRNG for yourself, which will produce numbers up to the maximum possible required. Use a single static copy of the built-in RNG to seed a new local copy of your own PRNG for each new thread you generate. Each thread-local PRNG will have its own local storage, and never needs to refer to the central RNG again.

This assumes that a statistically good RNG is fine for you and that cryptographic security is not an issue.

rossum
  • 15,344
  • 1
  • 24
  • 38
  • 5
    … ignoring the fact that writing a “statistically good” PRNG isn’t quite trivial. I strongly advise not to write one yourself (unless it’s for an exercise) and using an existing, quality-checked implementation. – Konrad Rudolph Jan 20 '14 at 16:17
  • @Konrad Rudolph: Copy one from the Internet. Marsaglia's KISS, or some of the papers by L'Ecuyer give some correct, fast and usable code. Apologies for not expressing myself more clearly. – rossum Jan 20 '14 at 16:44
  • 1
    That’s certainly better but to be honest, until I’m sure that this is the bottleneck I wouldn’t bother. MT is reasonably fast for most uses, [and C++ also implements good LCGs](http://en.cppreference.com/w/cpp/numeric/random/linear_congruential_engine). I’d go hunting for algorithms once I’ve established that these are not good enough. Not before. – Konrad Rudolph Jan 20 '14 at 16:53