[issue40286] Add randbytes() method to random.Random

[Raymond Hettinger]

Raymond Hettinger  added the comment:


New changeset f01d1be97d740ea0369379ca305646a26694236e by Raymond Hettinger in 
branch 'master':
bpo-40286: Put methods in correct sections. Add security notice to use secrets 
for session tokens. (GH-19870)
https://github.com/python/cpython/commit/f01d1be97d740ea0369379ca305646a26694236e


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[Raymond Hettinger]

Change by Raymond Hettinger :


--
pull_requests: +19183
pull_request: https://github.com/python/cpython/pull/19870

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[STINNER Victor]

STINNER Victor  added the comment:

It removed the C implementation of randbytes(): it was the root issue which 
started discussions here and in bpo-40346. I rejected bpo-40346 (BaseRandom) 
and related PRs.

I close the issue.

--
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 2d8757758d0d75882fef0fe0e3c74c4756b3e81e by Victor Stinner in 
branch 'master':
bpo-40286: Remove C implementation of Random.randbytes() (GH-19797)
https://github.com/python/cpython/commit/2d8757758d0d75882fef0fe0e3c74c4756b3e81e


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +19118
stage: resolved -> patch review
pull_request: https://github.com/python/cpython/pull/19797

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[STINNER Victor]

STINNER Victor  added the comment:

Raymond:
> The randbytes() method needs to depend on genrandbits().

I created PR 19700 which allows to keep the optimization (C implementation in 
_randommodule.c) and Random subclasses implement randbytes() with getrandbits().

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[STINNER Victor]

STINNER Victor  added the comment:

Raymond:
> Also, I don't want randbytes() in the C extension.  We're tried to keep as 
> much of the code as possible in pure Python and only have the MersenneTwister 
> specific code in the C module.  The improves maintainability and makes the 
> code more accessible to a broader audience.

I don't see how 30 lines makes Python so harder to maintain. These lines make 
the function 4x to 5x faster. We are not talking about 5% or 10% faster. I 
think that such optimization is worth it. When did we decide to stop optimizing 
Python?


Raymond:
> The randbytes() method needs to depend on genrandbits().

I created bpo-40346: "Redesign random.Random class inheritance" for a more 
generic fix, not just randbytes().


Raymond:
> Also, please don't change the name of the genrand_int32() function.  It was a 
> goal to change as little as possible from the official, standard version of 
> the C code at 
> http://www.math.sci.hiroshima-u.ac.jp/~m-mat/MT/MT2002/emt19937ar.html .

This code was already modified to replace "unsigned long" with "uint32_t" for 
example. I don't think that renaming genrand_int32() to genrand_uint32() makes 
the code impossible to maintain. Moreover, it seems like 
http://www.math.sci.hiroshima-u.ac.jp/~m-mat/MT/MT2002/emt19937ar.html was not 
updated for 13 years.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

$ ./python -m timeit -s 'import random' 'random.randbytes(10**6)'
200 loops, best of 5: 1.36 msec per loop

$ ./python -m timeit -s 'import random' 
'random.getrandbits(10**6*8).to_bytes(10**6, "little")'
50 loops, best of 5: 6.31 msec per loop

The Python implementation is only 5 times slower than the C implementation. I 
am fine with implementing randbytes() in Python. This would automatically make 
it depending on the getrandbits() implementation.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[STINNER Victor]

STINNER Victor  added the comment:

I created bpo-40346: "Redesign random.Random class inheritance".

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[Raymond Hettinger]

Raymond Hettinger  added the comment:

When a new method gets added to a module, it should happen in a way that is in 
harmony with the module's design.

--
nosy: +tim.peters

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[Raymond Hettinger]

Raymond Hettinger  added the comment:

Direct link to MT code that I would like to leave mostly unmodified:  
http://www.math.sci.hiroshima-u.ac.jp/~m-mat/MT/MT2002/CODES/mt19937ar.c

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[Raymond Hettinger]

Raymond Hettinger  added the comment:

The randbytes() method needs to depend on genrandbits().  It is documented that 
custom generators can supply there own random() and genrandbits() methods and 
expect that the other downstream generators all follow.  See the attached 
example which demonstrates that randbytes() bypasses this framework pattern.

Also, I don't want randbytes() in the C extension.  We're tried to keep as much 
of the code as possible in pure Python and only have the MersenneTwister 
specific code in the C module.  The improves maintainability and makes the code 
more accessible to a broader audience.

Also, please don't change the name of the genrand_int32() function.  It was a 
goal to change as little as possible from the official, standard version of the 
C code at 
http://www.math.sci.hiroshima-u.ac.jp/~m-mat/MT/MT2002/emt19937ar.html .  For 
the most part, we just want to wrap that code for Python bindings, but not 
modify it.

--
status: closed -> open
Added file: https://bugs.python.org/file49078/run_random.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 87502ddd710eb1f030b8ff5a60b05becea3f474f by Victor Stinner in 
branch 'master':
bpo-40286: Use random.randbytes() in tests (GH-19575)
https://github.com/python/cpython/commit/87502ddd710eb1f030b8ff5a60b05becea3f474f


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:


New changeset 223221b290db00ca1042c77103efcbc072f29c90 by Serhiy Storchaka in 
branch 'master':
bpo-40286: Makes simpler the relation between randbytes() and getrandbits() 
(GH-19574)
https://github.com/python/cpython/commit/223221b290db00ca1042c77103efcbc072f29c90


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +18915
pull_request: https://github.com/python/cpython/pull/19575

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
pull_requests: +18914
pull_request: https://github.com/python/cpython/pull/19574

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 9f5fe7910f4a1bf5a425837d4915e332b945eb7b by Victor Stinner in 
branch 'master':
bpo-40286: Add randbytes() method to random.Random (GH-19527)
https://github.com/python/cpython/commit/9f5fe7910f4a1bf5a425837d4915e332b945eb7b


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[STINNER Victor]

Change by STINNER Victor :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[STINNER Victor]

STINNER Victor  added the comment:

I wrote a quick benchmark:
---
import pyperf
import random

gen = random.Random()
# gen = random.SystemRandom()
gen.seed(850779834)

if 1: #hasattr(gen, 'randbytes'):
func = type(gen).randbytes
elif 0:
def py_randbytes(gen, n):
data = bytearray(n)
i = 0
while i < n:
chunk = 4
word = gen.getrandbits(32)
word = word.to_bytes(4, 'big')
chunk = min(n, 4)
data[i:i+chunk] = word[:chunk]
i += chunk
return bytes(data)

func = py_randbytes
else:
def getrandbits_to_bytes(gen, n):
return gen.getrandbits(n * 8).to_bytes(n, 'little')

func = getrandbits_to_bytes

runner = pyperf.Runner()
for nbytes in (1, 4, 16, 1024, 1024 * 1024):
runner.bench_func(f'randbytes({nbytes})', func, gen, nbytes)
---

Results on Linux using gcc -O3 (without LTO or PGO) using the C randbytes() 
implementation as the reference:

++-+--+---+
| Benchmark  | c_randbytes | py_randbytes | 
getrandbits_to_bytes  |
++=+==+===+
| randbytes(1)   | 71.4 ns | 1.04 us: 14.51x slower (+1351%)  | 244 ns: 
3.42x slower (+242%)  |
++-+--+---+
| randbytes(4)   | 71.4 ns | 1.03 us: 14.48x slower (+1348%)  | 261 ns: 
3.66x slower (+266%)  |
++-+--+---+
| randbytes(16)  | 81.9 ns | 3.07 us: 37.51x slower (+3651%)  | 321 ns: 
3.92x slower (+292%)  |
++-+--+---+
| randbytes(1024)| 1.05 us | 173 us: 165.41x slower (+16441%) | 3.66 
us: 3.49x slower (+249%) |
++-+--+---+
| randbytes(1048576) | 955 us  | 187 ms: 196.30x slower (+19530%) | 4.37 
ms: 4.58x slower (+358%) |
++-+--+---+

* c_randbytes: PR 19527, randbytes() methods implemented in C
* py_randbytes: bytearray, getrandbits(), .to_bytes()
* getrandbits_to_bytes: Serhiy's implementation: gen.getrandbits(n * 
8).to_bytes(n, 'little')

So well, the C randbytes() implementation is always the fastest.


random.SystemRandom().randbytes() (os.urandom(n)) performance using 
random.Random().randbytes() (Mersenne Twister) as a reference:

++-+-+
| Benchmark  | c_randbytes | systemrandom|
++=+=+
| randbytes(1)   | 71.4 ns | 994 ns: 13.93x slower (+1293%)  |
++-+-+
| randbytes(4)   | 71.4 ns | 1.04 us: 14.60x slower (+1360%) |
++-+-+
| randbytes(16)  | 81.9 ns | 1.02 us: 12.49x slower (+1149%) |
++-+-+
| randbytes(1024)| 1.05 us | 6.22 us: 5.93x slower (+493%)   |
++-+-+
| randbytes(1048576) | 955 us  | 5.64 ms: 5.91x slower (+491%)   |
++-+-+

os.urandom() is way slower than Mersenne Twister.

Well, that's not surprising: os.urandom() requires at least one syscall 
(getrandom() syscall on my Linux machine).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

All Random methods give the same result independently of endianess and bitness 
of the platform.

> I don't think that these two implementations give the same result on big and 
> little endian.

The second one does.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[STINNER Victor]

STINNER Victor  added the comment:

The performance of the new method is not my first motivation.

My first motivation is to avoid consumers of the random to write a wrong 
implementation which would be biased. It's too easy to write biased functions 
without notifying.

Moreover, it seems like we can do something to get reproducible behavior on 
different architectures (different endianness) which would also be a nice 
feature.

For example, in bpo-13396, Amaury found this two functions in the wild:

* struct.pack("Q", random.getrandbits(64))
* sha1(str(random.getrandbits(8*20))).digest()

As I wrote, users are creative to workaround missing features :-) I don't think 
that these two implementations give the same result on big and little endian.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40286] Add randbytes() method to random.Random

[STINNER Victor]

STINNER Victor  added the comment:

I updated my PR to rename the method to randbytes().

--
title: Add getrandbytes() method to random.Random -> Add randbytes() method to 
random.Random

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com