51

Ok. Here is what I know that won't work:

int Rand()
{
    //will return the same number over and over again
    return new Random().Next();
}

static Random rnd=new Random();

int Rand()
{
    //if used like this from multiple threads, rnd will dissintegrate 
    //over time and always return 0
    return rnd.Next();
}

This will work correctly, but if used by multiple threads, the CPU usage goes way up, which I don't want, and which I think is not necessary:

int Rand()
{
    lock(rnd)
    {
        return rnd.Next();
    }
}

So, is there a thread-safe Random class for c#, or a better way to use it?

Arsen Zahray
  • 24,367
  • 48
  • 131
  • 224
  • 2
    Try a `ThreadLocal`. `private static readonly ThreadLocal rand = new ThreadLocal(() => new Random());` – Jeroen Vannevel Oct 09 '13 at 11:22
  • 7
    @JeroenVannevel: You will probably want to randomize the seeds of the `Random` instances, because otherwise threads starting at the same time would get the same sequence of random numbers. – dtb Oct 09 '13 at 11:24
  • .. and take a global locked static random instance for the seed of the ThreadLocal random instances. – Ralf Oct 09 '13 at 11:24
  • 5
    CPU usage goes way up with a lock? That's a good sign: the lock isn't a bottleneck, and your threads are doing work. – Julien Lebosquain Oct 09 '13 at 11:24

4 Answers4

78

I use something like this:

public static class StaticRandom
{
    static int seed = Environment.TickCount;

    static readonly ThreadLocal<Random> random =
        new ThreadLocal<Random>(() => new Random(Interlocked.Increment(ref seed)));

    public static int Rand()
    {
        return random.Value.Next();
    }
}
Alessandro D'Andria
  • 8,663
  • 2
  • 36
  • 32
  • I really like this, it works correctly, and when used in multithread, this example is 10 times faster than the lock method! – Arsen Zahray Oct 09 '13 at 11:59
  • 3
    Won't this risk producing a bunch of Random instances with the same seed, when threads are created and start using it about the same time? – Eric J. Jun 15 '16 at 18:04
  • 6
    @EricJ. no, because it uses an incrementing seed. – Cory Nelson Aug 18 '16 at 21:08
  • 2
    I missed the Increment. Still, this would suffer from predictability. The first seed is based on the tick count, therefore all subsequent seeds could be guessed. An early release of Netscape's SSL implementation was broken due to a random value based on a timestamp. True non-predictability is very non-trivial without a hardware entropy generator. – Eric J. Aug 21 '16 at 21:33
  • 18
    The `Random` class was never designed for non-predictability in the first place. For secure random numbers you'll need a `RandomNumberGenerator` from `System.Security.Cryptography` instead. – piedar Jan 15 '17 at 16:11
  • So this will create one `Random` per thread. Since they are static are they ever removed when threads are completed? Or they will be reused when a thread is again picked from the pool? My question is, are we gonna end up with millions of `Random` objects using this? – Magnus May 09 '22 at 09:35
  • @Magnus the `Random` per thread will be collected with the thread itself, so you don't have to worry. – Alessandro D'Andria May 09 '22 at 12:37
  • I fail to understand how a monotonically incrementing seed is better than the default constructor or even just TickCount...(yes I realize they could be created at the same instant and have the same value for "ticks" but srsly if you're doing it this way you're not serious about randomness anyway so..?) – tekHedd Jun 08 '23 at 23:17
14
readonly ThreadLocal<Random> random = 
    new ThreadLocal<Random>(() => new Random(GetSeed()));

int Rand()
{
    return random.Value.Next();
}

static int GetSeed()
{
    return Environment.TickCount * Thread.CurrentThread.ManagedThreadId;
}

(shamelessly stolen from the comment of Jeroen Vannevel)

Benoit Blanchon
  • 13,364
  • 4
  • 73
  • 81
  • I've no shame to upvote, first arrived, first served :) – AlexH Oct 09 '13 at 11:50
  • 3
    There is no shame in improving suggestions! ;) – Jeroen Vannevel Oct 09 '13 at 12:10
  • 3
    multiplying time with thread ID is silly. Consider the cases where one of them is a 0, or even just a power-of-two. And even if that weren't an issue, generating a random 31 bit seed for each instance hits you even harder with the birthday problem than Alessandro. – CodesInChaos Oct 09 '13 at 16:22
6

I think what you want is threadstatic

[ThreadStatic]
static Random rnd=new Random();

int Rand()
{
    if ( rnd == null ) 
    {
       rnd = new Random()
    }
    //Now each thread gets it's own version
    return rnd.Next();
}

That way each thread get their own version of your rnd property

The reason your locking will increase cpu usage is because all threads will wait on that single point (should only be an issue if you use it a lot)

[Update] i fixed the initialization. As someone pointed out it does leave the fact that if you start multiple threads in the same milisecond then they will produce the same results.

Batavia
  • 2,497
  • 14
  • 16
  • 4
    According to this answer http://stackoverflow.com/a/18337158/1164966, `ThreadLocal` is to be prefered over `[ThreadStatic]` because it garanties that the field is initialized for every threads. – Benoit Blanchon Oct 09 '13 at 11:33
  • Or if you dislike ThreadStatic, then explicitly create as many Random() instances as you have theads. – Eiver Oct 09 '13 at 11:33
  • 4
    It's risky anyway. If you start two threads in quick succession, their seeds can be the same. – harold Oct 09 '13 at 11:35
  • testing it. if you initialize rnd like that, it'll cause errors – Arsen Zahray Oct 09 '13 at 11:57
  • 1
    This answer is incorrect. `new Random()` uses the current time (in ms) as its seed. If two instances of `Random` are created in the same millisecond *(which is not hard to do, 1 ms is [an eternity in computer time](http://www.blueraja.com/blog/368/how-fast-are-computers-in-everyday-terms))*, they will produce exactly the same random numbers. – BlueRaja - Danny Pflughoeft Oct 09 '13 at 12:21
  • If you use a `[ThreadStatic]` field, you can't use a field initializer. You need to check for `null` in each call to `Rand`. – CodesInChaos Oct 09 '13 at 16:18
  • Do not specify initial values for fields marked with ThreadStaticAttribute, because such initialization occurs only once, when the class constructor executes, and therefore affects only one thread. https://learn.microsoft.com/en-us/dotnet/api/system.threadstaticattribute?view=net-6.0 – obratim Aug 02 '22 at 17:21
1

My group recently looked into this. We came to the conclusion that we should use a random number generator that has been specifically designed to support parallel computing. Tina's Random Number Generator Library (http://numbercrunch.de/trng/) has a stable implementation, and a manual with a theoretical introduction and references to the relevant literature. So far, we are very satisfied with it.

Joachim W
  • 7,290
  • 5
  • 31
  • 59