[issue42160] unnecessary overhead in tempfile

[STINNER Victor]

Change by STINNER Victor :


--
nosy:  -vstinner

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Inada Naoki]

Change by Inada Naoki :


--
pull_requests: +21987
pull_request: https://github.com/python/cpython/pull/23068

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

I have tested my idea, such optimization speeds up _RandomNameSequence by 16%. 
At the cost of more complex code. It is not worth.

--
resolution:  -> rejected
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

[issue42160] unnecessary overhead in tempfile

[Eric Wolf]

Eric Wolf  added the comment:

>>> timeit(os.getpid)
0.0899073329931

Considering the reference leaks, os.getpid() seems to be the better solution.

--

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Instead of os.getpid() we can use a global variable, and the single callback 
can reset it.

But is it worth? Is os.getpid() so slow?

--

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Eric Wolf]

Eric Wolf  added the comment:

It would be possible to allow the GC to finalize the Random instances through 
weak references.
On the other hand, if many _RandomNameSequence instances were used temporarily, 
a lot of callbacks would be registered via os.register_at_fork(), could that 
cause problems?

--

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

See also issue30030.

--
nosy: +serhiy.storchaka

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Inada Naoki]

Inada Naoki  added the comment:

@Deric-W Please create another PR to change from `choise` to `choises` if you 
want to optimize it.

--

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Inada Naoki]

Inada Naoki  added the comment:


New changeset 43ca084c88d1e46a44199f347c72e26db84903c9 by Inada Naoki in branch 
'master':
Revert "bpo-42160: tempfile: Reduce overhead of pid check. (GH-22997)"
https://github.com/python/cpython/commit/43ca084c88d1e46a44199f347c72e26db84903c9


--

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Inada Naoki]

Change by Inada Naoki :


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

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Inada Naoki]

Inada Naoki  added the comment:

Oh, _RandomNameSequence is not true singleton. Instance is used in tests 
actually.
Using `os.register_at_fork` was but idea. Let's reject it.

--

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[STINNER Victor]

STINNER Victor  added the comment:

The commit 8e409cebad42032bb7d0f2cadd8b1e36081d98af introduced a reference leak:
https://buildbot.python.org/all/#/builders/320/builds/71

$ make && ./python -m test test_tempfile -R 3:3
...
test_tempfile leaked [49, 49, 49] references, sum=147
test_tempfile leaked [28, 28, 28] memory blocks, sum=84
...

Single test reproducing the leak:

$ ./python -m test test_tempfile -R 3:3 -m 
test.test_tempfile.TestGetDefaultTempdir.test_no_files_left_behind
...
test_tempfile leaked [21, 21, 21] references, sum=63
test_tempfile leaked [12, 12, 12] memory blocks, sum=36
...

--
nosy: +vstinner
resolution: fixed -> 
status: closed -> open

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Eric Wolf]

Eric Wolf  added the comment:

Thanks

--

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Inada Naoki]

Inada Naoki  added the comment:

Thanks

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed
versions: +Python 3.10 -Python 3.6, Python 3.7, Python 3.8, Python 3.9

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Inada Naoki]

Inada Naoki  added the comment:


New changeset 8e409cebad42032bb7d0f2cadd8b1e36081d98af by Eric W in branch 
'master':
bpo-42160: tempfile: Reduce overhead of pid check. (GH-22997)
https://github.com/python/cpython/commit/8e409cebad42032bb7d0f2cadd8b1e36081d98af


--
nosy: +methane

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Eric Wolf]

Eric Wolf  added the comment:

It seems to be insignificant, however it would allow for easier 
monkey-patching: https://bugs.python.org/issue32276

Instead of changing _Random one could simply assign a new instance to 
_named_sequence

--

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Benjamin Peterson]

Benjamin Peterson  added the comment:

I also wonder if the overhead of getpid() is actually significant for anything.

--

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Eric Wolf]

Eric Wolf  added the comment:

SystemRandom seems to be slower:


from random import Random, SystemRandom
from timeit import timeit

user = Random()
system = SystemRandom()
characters = "abcdefghijklmnopqrstuvwxyz0123456789_"

timeit(lambda: user.choice(characters))
>>> 0.5491522020020057

timeit(lambda: system.choice(characters))
>>> 2.9195130389998667

--

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Benjamin Peterson]

Benjamin Peterson  added the comment:

Wouldn't it be simpler to use random.SystemRandom instead?

--
nosy: +benjamin.peterson

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Eric Wolf]

Change by Eric Wolf :


--
keywords: +patch
pull_requests: +21911
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/22997

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Eric Wolf]

New submission from Eric Wolf :

The tempfile module contains the class _RandomNameSequence, which has the rng 
property.
This property checks os.getpid() every time and re-initializes a random number 
generator when it has changed.
However, this is only necessary on systems which allow the process to be cloned 
and should be solved on such systems with os.register_at_fork (see the random 
module).

--

___
Python tracker 

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

[issue42160] unnecessary overhead in tempfile

[Eric Wolf]

Change by Eric Wolf :


--
components: Library (Lib)
nosy: Deric-W
priority: normal
severity: normal
status: open
title: unnecessary overhead in tempfile
type: enhancement
versions: Python 3.6, Python 3.7, Python 3.8, Python 3.9

___
Python tracker 

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