Skip to content

bpo-36559: random: import hashlib on demand#12728

Closed
vstinner wants to merge 2 commits into
python:masterfrom
vstinner:random_import_hashlib
Closed

bpo-36559: random: import hashlib on demand#12728
vstinner wants to merge 2 commits into
python:masterfrom
vstinner:random_import_hashlib

Conversation

@vstinner

@vstinner vstinner commented Apr 8, 2019

Copy link
Copy Markdown
Member

"import random" no longer imports the hashlib module by default.
hashlib is now imported on demand, to hash a string seed in
Random.seed().

https://bugs.python.org/issue36559

"import random" no longer imports the hashlib module by default.
hashlib is now imported on demand, to hash a string seed in
Random.seed().

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but

import hashlib
...
hashlib.sha512()

can be a tiny bit faster.

@vstinner

vstinner commented Apr 8, 2019

Copy link
Copy Markdown
Member Author

LGTM, but import hashlib ... hashlib.sha512() can be a tiny bit faster.

Well, I wanted to minimize changes. I'm surprised that it's faster. Is it really faster? :-) To be honest, I have no preference.

@vstinner

vstinner commented Apr 9, 2019

Copy link
Copy Markdown
Member Author

I ran a quick micro-benchmark:

$ python3 -m perf timeit -o from.json -v -s 'import hashlib' 'from hashlib import sha256'
$ python3 -m perf timeit -o import.json -v -s 'import hashlib' 'import hashlib; hashlib.sha256'
$ python3 -m perf compare_to import.json from.json 
Mean +- std dev: [import] 123 ns +- 3 ns -> [from] 684 ns +- 10 ns: 5.54x slower (+454%)

Oh, I'm surprised by these numbers. So yeah, "import hashlib; hashlib.sha256" is near 6x faster than "from hashlib import sha256". It doesn't make sense to me, since the 2 code should be the same.

@tiran

tiran commented Apr 9, 2019

Copy link
Copy Markdown
Member

I have created the alternative PR #12742. It's a good compromise between Victor's need for speed and Raymond's wish for simple code.

@serhiy-storchaka

Copy link
Copy Markdown
Member

It doesn't make sense to me, since the 2 code should be the same.

They are not the same. In the second case it calls the Python code to handle non-empty from list.

@vstinner

Copy link
Copy Markdown
Member Author

@rhettinger merged PR #12742, so I close my PR.

@vstinner vstinner closed this Apr 10, 2019
@vstinner
vstinner deleted the random_import_hashlib branch April 10, 2019 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants