Comment 3 for bug 1412488

Revision history for this message
Axel Schwenke (ahel) wrote :

> the patch uses SB_MAX_RND (0x3fffffff) as the modulus, while
> the chosen values of multiplier and increment correspond to
> the modules value of 2^32. Which means condition number #2 from
> https://en.wikipedia.org/wiki/Linear_congruential_generator#Period_length
> does not hold, and thus the chosen constants may have a negative
> impact on RNG quality.

I think this is not a valid point for two reasons:

1. SB_MAX_RND is *not* the modulus of the LCG. The modulus is 2^32, but output of the LCG is projected to a smaller range by the (seed % SB_MAX_RND) operation. There is no way that this could affect the resulting period length to become shorter than SB_MAX_RND itself.

2. the current implementation does the same thing. In sysbench.h there is

#define SB_MAX_RND 0x3fffffffu
...
#ifdef HAVE_LRAND48
# define sb_rnd() (lrand48() % SB_MAX_RND)
...

meaning the 31 random bits returned from lrand48() are projected to a smaller range.

Further more I think the modulus operation above is wrong. If we want to project a random number from a larger range inside the interval [0..SB_MAX_RND] (limits included) then we should use (lrand48() % (SB_MAX_RND+1)). Which is also more efficient because it can be implemented as (rand_input & SB_MAX_RND), thus avoiding a division.

I didn't read all the code, but do you remember why there is a fixed value of SB_MAX_RND at all? The only place in the code where I see it being used is when the result from sb_rnd() is transformed to a DOUBLE value in [0..1]. Wouldn't it be much better to chose SB_MAX_RND according to the RNG that is picked by autoconf? Then SB_MAX_RND would be 2^32-1 for lrand48() (which return 31 bits) and 2^33-1 for sb_rnd_local() (which returns 32 bits).