Generating random numbers in multiple threads

I'm trying to learn how to use threads to generate lots of random numbers. In particular, I'd like know:

-
Is the following code thread safe? (I think the histogram class is the only one that requires special attention)

-
Are there implication for the 'randomness' of my numbers when they are generated from different threads?

-
Is my timing giving me meaningful results?

Currently, I can't use a std::vector as my container type, because my histogram class relies on the container being initialised with zeroed elements. Is it possible to allow both fixed length and variable length containers?

```
#include
#include
#include
#include
#include
#include
#include
#include

//class to produce random numbers from a poisson distribution
class poisson_generator
{
public:
poisson_generator( int seed, double mean)
:engine{ seed }, poisson{ mean } {}

int operator()()
{ return sample(); }

int sample()
{ return poisson(engine) ; }

private:
std::default_random_engine engine;
std::poisson_distribution poisson;

};

//class to store random numbers
template
class histogram
{
public:
histogram()
:fData{} {}

void fill( int val )
{
std::lock_guard guard(fMutex);
++fData[val];
}

typename Container::iterator begin()
{ return fData.begin(); }

typename Container::iterator end()
{ return fData.end(); }

typename Container::value_type at(typename Container::size_type pos)
{
std::lock_guard guard(fMutex);
return fData.at(pos);
}

private:
Container fData;
std::mutex fMutex;
};

//Function to gnenerate events
template
void generate_events( Generator& g , Storage& store, int nEvents )

{
std::cout
void printHistogram( H& h, unsigned int min, unsigned int max, int peak = 30 )
{

auto total = std::accumulate( begin(h), end

There are a couple of things to worry about wrt to shared memory.

First, histogram::begin and histogram::end provide unguarded access to the container. You're using them safely here, but you should generally not provide such unguarded access. One possible solution is a friend histogram_range, which has a std::lock_guard against fMutex as a member and provides begin and end functions that return Container::const_iterator.

Second, you provide shared access to rd. The standard doesn't require (as far as I can tell) that rd::operator() be thread-safe. Better would be to add the following line to your for loop: const auto seed = rd(); and capture seed instead of &rd in the lambda expression.

Otherwise, your thread safety seems OK.

I can't imagine that generating random numbers from different threads would have a deleterious effect on the randomness of the numbers. In fact, the excellent ` primer released by the C++ standards committee explicitly mentions (approvingly) that applications may create thread-local engines (see footnote 14 on pg 5).

You appear to be measuring time correctly (except see below), and in particular you appear to be printing the number of milliseconds that your program takes to generate the random data you care about. Beyond that, I'm not sure what you mean by meaningful.

std::monotonic_clock was present in some drafts, but was removed from the final standard. Use std::steady_clock instead.

Some general notes:

In histogram: I suggest that at be declared const, and fMutex be declared mutable. Read-only access to a const histogram& should be possible.

Your code has undefined behavior whenever your generator generates a number larger than 100 (because you use a std::array as your backing store). Consider using at instead of operator[] to access the data, as at does bounds checking. Probably a better solution is to use a std::vector as your storage instead of templatizing on the Container type, and doing explicit bounds checking yourself in fill. You can then expand the vector` (e.g. with

if (val > fData.size()) fData.insert(fData.end(), val - fData.size(), 0);

Your spacing and indentation are inconsistent. That makes your code more difficult to read.

StackExchange Code Review Q#28650, answer score: 4