Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

subprocess: Prefer close_range() to procfs-based fd closing #92301

Closed
izbyshev opened this issue May 4, 2022 · 1 comment
Closed

subprocess: Prefer close_range() to procfs-based fd closing #92301

izbyshev opened this issue May 4, 2022 · 1 comment
Labels
performance Performance or resource usage type-feature A feature request or enhancement

Comments

@izbyshev
Copy link
Contributor

izbyshev commented May 4, 2022

subprocess gained the ability to use close_range() in #84603, however, it's only used as a fallback if procfs is not available, which means that at least on Linux it's almost never used. Its performance benefits are significant (see numbers at the end). I propose to use it by default, with procfs being the second option.

This requires some extra changes because _Py_closerange() was designed to be standalone and tries all available methods, including brute force, and also because it's not required to be async-signal-safe, but we must preserve async-signal-safety of fd closing code on Linux. I'll open a PR with a proposed implementation.

Performance comparison that I did on a test version with the following script (may require you to raise the fd limit in your shell):

import os, subprocess, sys, timeit
from resource import *

soft, hard = getrlimit(RLIMIT_NOFILE)
setrlimit(RLIMIT_NOFILE, (hard, hard))

num_fds, num_iter = map(int, sys.argv[1:3])

for i in range(num_fds):
    os.open('/dev/null', os.O_RDONLY)

print(timeit.timeit(lambda: subprocess.run('/bin/true'), number=num_iter))

Before:

./python test.py 100 100
0.11497399304062128
./python test.py 1000 100
0.3999998280778527
./python test.py 10000 100
3.19203062588349

After:

./python test.py 100 100
0.07936713611707091
./python test.py 1000 100
0.09550889488309622
./python test.py 10000 100
0.288825839292258
@izbyshev izbyshev added the type-feature A feature request or enhancement label May 4, 2022
izbyshev added a commit to izbyshev/cpython that referenced this issue May 4, 2022
…losing

close_range() is much faster for large number of file descriptors, e.g.
4 times faster for 1000 descriptors in a Linux 5.16-based environment.
@hugovk hugovk added the performance Performance or resource usage label May 4, 2022
izbyshev added a commit to izbyshev/cpython that referenced this issue May 4, 2022
…losing

close_range() is much faster for large number of file descriptors, e.g.
4 times faster for 1000 descriptors in a Linux 5.16-based environment.
gpshead pushed a commit that referenced this issue May 5, 2022
…#92303)

#92301: subprocess: Prefer `close_range()` to procfs-based fd closing.

`close_range()` is much faster for large number of file descriptors, e.g.
4 times faster for 1000 descriptors in a Linux 5.16-based environment.

We prefer close_range() only if it's known to be async-signal-safe.
@gpshead
Copy link
Member

gpshead commented May 5, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance or resource usage type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

3 participants