Issue 23488: Random objects twice as big as necessary on 64-bit builds

The Modules/_randommodule.c implements the 32-bit version of the MersenneTwister and its struct uses (unsigned long) for each of the 624 elements of the state vector.  

On a 32-bit build, the unsigned longs are 4 bytes.  However, on a 64-bit build, they are 8 bytes each eventhough only the bottom 32-bits are used.  This causes the random object to be twice as big as necessary. sys.getsizeof(_random.Random()) reports 5016 bytes.  This wastes memory, grinds the cache, and slows performance.

The (unsigned long) declaration should probably be replaced with (uint32_t).

> The (unsigned long) declaration should probably be replaced with (uint32_t)

Would it be possible to benchmark this change, to ensure that it doesn't kill performances?

A quick micro-benchmark using timeit should be enough ;)

I agree with the change, I already noticed the unused bits long time ago, when I took at look at the Mersenne Twister implementation.

Oh, by the way, using 32 bits unsigned integers would avoid all the "& 0xffffffff" everywhere.

Yes, I noticed this when reimplementing the random module in Numba.
*Theoretically*, I think you need "long" to ensure ints are at least 32 bits. But in practice, I think CPython already needs 32-bit C ints.

(note Numpy also uses C longs internally)

> Would it be possible to benchmark this change, to ensure that it doesn't kill performances?

There is no way it can kill performance.

Here is a patch. It also optimizes getrandbit() and seed() as was originally proposed in issue16496.

> Here is a patch.

Where?!

BYW, we might want to use PY_UINT32_T rather than uint32_t directly.

Oh, sorry, here is it.

Some microbenchmark results on 32-bit Linux:

$ ./python -m timeit -s "from random import getrandbits" -- "getrandbits(64)"
Before: 1000000 loops, best of 3: 1.41 usec per loop
After:  1000000 loops, best of 3: 1.34 usec per loop

$ ./python -m timeit -s "from random import getrandbits" -- "getrandbits(2048)"
Before: 100000 loops, best of 3: 5.84 usec per loop
After:  100000 loops, best of 3: 5.61 usec per loop

$ ./python -m timeit -s "from random import getrandbits" -- "getrandbits(65536)"
Before: 10000 loops, best of 3: 145 usec per loop
After:  10000 loops, best of 3: 137 usec per loop

The patch looks good to me.

For utint32_t, see my old issue #17884: "Try to reuse stdint.h types like int32_t".

> see my old issue #17884

... and you can also re-read my explanations in that issue about why simply using uint32_t and int32_t doesn't work!  We need something like PY_UINT32_T (and co) for portability.

The only part of #17884 that's still valid is that it may well make sense to insist that exact-width 32-bit and 64-bit signed and unsigned integer types are available when building Python.

Updated patch addresses some Victor's comments.

Ping.

You should fix the comment as mentioned in the review, otherwise looks good to me.

This is good to go.

This would have gone quicker if the size bug-fix hadn't been commingled with the optimization.

New changeset 4b5461dcd190 by Serhiy Storchaka in branch 'default':
Issue #23488: Random generator objects now consume 2x less memory on 64-bit.
https://hg.python.org/cpython/rev/4b5461dcd190

New changeset 16d0e3dda31c by Zachary Ware in branch 'default':
Issue #23488: Fix a syntax error on big endian platforms.
https://hg.python.org/cpython/rev/16d0e3dda31c

Thanks Zachary for fixing this.

Should this be backported?  IMO, it is a bug.

messages: + msg239379