0

From Fastest way to generate a random boolean, in the comment, CodesInChaos said:

MS messed up the implementation of NextBytes, so it's surprisingly slow.

[...] the performance is about as bad as calling Next for each byte, instead of taking advantage of all 31 bits. But since System.Random has bad design and implementation at pretty much every level, this is one of my smaller gripes.

Why did he said MS has made a design and implementation at pretty much every level?

How is the Random class wrongly implemented?

Community
  • 1
  • 1
MiP
  • 5,846
  • 3
  • 26
  • 41
  • 2
    The question in your title is answered by your own quote. The question in your body is a separate question. –  May 28 '17 at 15:28
  • @hvd After reading .NET Reference Source, I'm not sure why he said it was slow: `for (int i=0; i – MiP May 28 '17 at 15:41
  • 4
    Microsoft did not design the algorithm, they got it from Donald Knuth's The Art of Programming. Programmers complain about their random number generator like farmers complain about the weather. – Hans Passant May 28 '17 at 15:50

1 Answers1

3

Of course I can't look inside his head for his reasons, but System.Random is pretty weird.

InternalSample() returns a non-negative int that cannot be int.MaxValue. That doesn't sound so bad on the outset, but that means it has almost (but not quite) 31 usable bits of randomness. That complicates things such as efficiently implementing NextBytes(byte[] buffer).. which it doesn't even try! It does this:

for (int index = 0; index < buffer.Length; ++index)
    buffer[index] = (byte) (this.InternalSample() % 256);

Making approximately 4 times more calls to InternalSample than necessary. Also the % 256 is useless, casting to a byte truncates anyway. It's also biased, 255 is just slightly less probable than any other result, since the internal sample cannot be int.MaxValue.

But it gets worse. For example, NextDouble uses this.InternalSample() * 4.6566128752458E-10. It is probably not immediately obvious, but 4.6566128752458E-10 is 1.0 / int.MaxValue. What's annoying about that is that it's not a power of two, so it's a "messy" number that causes the gaps between adjacent possible results to be nonuniform.

Worse yet, the algorithms for Next(int) and Next(int, int) are inherently biased, since they simply scale a random double and reject nothing. It's also not especially fast, which would normally be a reason to avoid rejection sampling.

It's also fairly slow in general. It's a subtractive generator, a relatively unknown PRNG that apparently isn't too bad, but it has a big state (which is slow to seed and has an annoying cache footprint) and a bunch of annoying operations in the sampling algorithm. Certainly it has better quality than a basic LCG, but it's significantly marred by biased scaling methods and bad performance.

The interface design is also annoying. Since the upper bounds everywhere are exclusive, there is no easy way to generate a sample in [0 .. int.MaxValue] or [int.MinValue .. int.MaxValue], both of which are fairly commonly useful. Exclusive upper bounds are often nice to avoid weird -1's, offering no way to get a full-range sample is just annoying. Of course it can be done through NextDouble, but since the input of NextDouble already isn't a full-range sample, the result is necessarily biased.

There are probably some deficiencies that I've missed.

harold
  • 61,398
  • 6
  • 86
  • 164
  • 1
    "For example, `NextDouble` uses `this.InternalSample() * 4.6566128752458E-10`. It is probably not immediately obvious, but `4.6566128752458E-10` is `1.0 / int.MaxValue`." -- The code has `return (InternalSample()*(1.0/MBIG));` with `private const int MBIG = Int32.MaxValue;`. If you were looking at decompiler output, I recommend the reference source. –  May 28 '17 at 16:07