Commit 461d6966 authored by Jason Madden's avatar Jason Madden

Use /dev/fd|/proc/self/fd to get open FDs to close in Popen

If those aren't available, use the old brute-force approach. This is
closer to what CPython does in its C implementation, and is much
faster.

We don't have to worry about the async signal safe stuff the C code
does because, guess what, we're running Python code here already
anyway, so much of it could wind up doing something that's not
actually safe anyway. Oh well.

Since we depend on Python 3.4 and above now, we can rely on the
CLOEXEC flag being set by default and not have to manually check
everything.

This speeds up 2.7 (close_fds defaults to *false* there, so the
default case doesn't change):

| Benchmark                 | 27_bench_subprocess | 27_bench_subprocess_dirfd     |
+---------------------------+---------------------+-------------------------------+
| spawn native no close_fds | 1.81 ms             | 1.79 ms: 1.01x faster (-1%)   |
| spawn gevent no close_fds | 2.11 ms             | 2.20 ms: 1.04x slower (+4%)   |
| spawn native close_fds    | 31.0 ms             | 30.2 ms: 1.03x faster (-3%)   |
| spawn gevent close_fds    | 31.6 ms             | 2.56 ms: 12.31x faster (-92%) |

And it really speeds up 3.7 (close_fds defaults to *true* there, so
the default case is much faster, and the non-default case is even better):

| Benchmark                 | 37_bench_subprocess | 37_bench_subprocess_dirfd     |
+---------------------------+---------------------+-------------------------------+
| spawn native no close_fds | 1.34 ms             | 1.27 ms: 1.06x faster (-6%)   |
| spawn gevent no close_fds | 117 ms              | 3.05 ms: 38.27x faster (-97%) |
| spawn native close_fds    | 1.36 ms             | 1.30 ms: 1.04x faster (-4%)   |
| spawn gevent close_fds    | 32.5 ms             | 3.34 ms: 9.75x faster (-90%)  |

Fixes #1172
parent b166aaf8
...@@ -10,6 +10,18 @@ ...@@ -10,6 +10,18 @@
- On Windows, CFFI is now a dependency so that the libuv backend - On Windows, CFFI is now a dependency so that the libuv backend
really can be used by default. really can be used by default.
- Fix a bug detecting whether we can use the memory monitoring
features when psutil is not installed.
- `gevent.subprocess.Popen` uses ``/proc/self/fd`` (on Linux) or
``/dev/fd`` (on BSD, including macOS) to find the file descriptors
to close when ``close_fds`` is true. This matches an optimization
added to Python 3 (and backports it to Python 2.7), making process
spawning up to 9 times faster. Also, on Python 3, since Python 3.3
is no longer supported, we can also optimize the case where
``close_fds`` is false (not the default), making process spawning up
to 38 times faster. Initially reported in :issue:`1172` by Ofer Koren.
1.3b1 (2018-04-13) 1.3b1 (2018-04-13)
================== ==================
......
...@@ -379,6 +379,12 @@ if 'TimeoutExpired' not in globals(): ...@@ -379,6 +379,12 @@ if 'TimeoutExpired' not in globals():
(self.cmd, self.timeout)) (self.cmd, self.timeout))
if hasattr(os, 'set_inheritable'):
_set_inheritable = os.set_inheritable
else:
_set_inheritable = lambda i, v: True
class Popen(object): class Popen(object):
""" """
The underlying process creation and management in this module is The underlying process creation and management in this module is
...@@ -1145,42 +1151,70 @@ class Popen(object): ...@@ -1145,42 +1151,70 @@ class Popen(object):
self._set_cloexec_flag(w) self._set_cloexec_flag(w)
return r, w return r, w
def _close_fds(self, keep): def _close_fds(self, keep, errpipe_write):
# `keep` is a set of fds, so we # From the C code:
# use os.closerange from 3 to min(keep) # errpipe_write is part of keep. It must be closed at
# and then from max(keep + 1) to MAXFD and # exec(), but kept open in the child process until exec() is
# loop through filling in the gaps. # called.
# Under new python versions, we need to explicitly set
# passed fds to be inheritable or they will go away on exec if os.path.isdir('/proc/self/fd'): # Linux
if hasattr(os, 'set_inheritable'): self._close_fds_from_path('/proc/self/fd', keep, errpipe_write)
set_inheritable = os.set_inheritable elif os.path.isdir('/dev/fd'): # BSD, including macOS
self._close_fds_from_path('/dev/fd', keep, errpipe_write)
else: else:
set_inheritable = lambda i, v: True self._close_fds_brute_force(keep, errpipe_write)
if hasattr(os, 'closerange'):
keep = sorted(keep)
min_keep = min(keep)
max_keep = max(keep)
os.closerange(3, min_keep)
os.closerange(max_keep + 1, MAXFD)
for i in xrange(min_keep, max_keep):
if i in keep:
set_inheritable(i, True)
continue
try: def _close_fds_from_path(self, path, keep, errpipe_write):
os.close(i) # path names a directory whose only entries have
except: # names that are ascii strings of integers in base10,
pass # corresponding to the fds the current process has open
try:
fds = [int(fname) for fname in os.listdir(path)]
except (ValueError, OSError):
self._close_fds_brute_force(keep, errpipe_write)
else: else:
for i in xrange(3, MAXFD): for i in keep:
if i in keep: if i == errpipe_write:
set_inheritable(i, True) continue
_set_inheritable(i, True)
for fd in fds:
if fd in keep or fd < 3:
continue continue
try: try:
os.close(i) os.close(fd)
except: except:
pass pass
def _close_fds_brute_force(self, keep, errpipe_write):
# `keep` is a set of fds, so we
# use os.closerange from 3 to min(keep)
# and then from max(keep + 1) to MAXFD and
# loop through filling in the gaps.
# Under new python versions, we need to explicitly set
# passed fds to be inheritable or they will go away on exec
# XXX: Bug: We implicitly rely on errpipe_write being the largest open
# FD so that we don't change its cloexec flag.
assert hasattr(os, 'closerange') # Added in 2.7
keep = sorted(keep)
min_keep = min(keep)
max_keep = max(keep)
os.closerange(3, min_keep)
os.closerange(max_keep + 1, MAXFD)
for i in xrange(min_keep, max_keep):
if i in keep:
_set_inheritable(i, True)
continue
try:
os.close(i)
except:
pass
def _execute_child(self, args, executable, preexec_fn, close_fds, def _execute_child(self, args, executable, preexec_fn, close_fds,
pass_fds, cwd, env, universal_newlines, pass_fds, cwd, env, universal_newlines,
startupinfo, creationflags, shell, startupinfo, creationflags, shell,
...@@ -1235,6 +1269,13 @@ class Popen(object): ...@@ -1235,6 +1269,13 @@ class Popen(object):
raise raise
if self.pid == 0: if self.pid == 0:
# Child # Child
# XXX: Technically we're doing a lot of stuff here that
# may not be safe to do before a exec(), depending on the OS.
# CPython 3 goes to great lengths to precompute a lot
# of this info before the fork and pass it all to C functions that
# try hard not to call things like malloc(). (Of course,
# CPython 2 pretty much did what we're doing.)
try: try:
# Close parent's pipe ends # Close parent's pipe ends
if p2cwrite != -1: if p2cwrite != -1:
...@@ -1254,16 +1295,16 @@ class Popen(object): ...@@ -1254,16 +1295,16 @@ class Popen(object):
errwrite = os.dup(errwrite) errwrite = os.dup(errwrite)
# Dup fds for child # Dup fds for child
def _dup2(a, b): def _dup2(existing, desired):
# dup2() removes the CLOEXEC flag but # dup2() removes the CLOEXEC flag but
# we must do it ourselves if dup2() # we must do it ourselves if dup2()
# would be a no-op (issue #10806). # would be a no-op (issue #10806).
if a == b: if existing == desired:
self._set_cloexec_flag(a, False) self._set_cloexec_flag(existing, False)
elif a != -1: elif existing != -1:
os.dup2(a, b) os.dup2(existing, desired)
try: try:
self._remove_nonblock_flag(b) self._remove_nonblock_flag(desired)
except OSError: except OSError:
# Ignore EBADF, it may not actually be # Ignore EBADF, it may not actually be
# open yet. # open yet.
...@@ -1296,21 +1337,7 @@ class Popen(object): ...@@ -1296,21 +1337,7 @@ class Popen(object):
if close_fds: if close_fds:
fds_to_keep = set(pass_fds) fds_to_keep = set(pass_fds)
fds_to_keep.add(errpipe_write) fds_to_keep.add(errpipe_write)
self._close_fds(fds_to_keep) self._close_fds(fds_to_keep, errpipe_write)
elif hasattr(os, 'get_inheritable'):
# close_fds was false, and we're on
# Python 3.4 or newer, so "all file
# descriptors except standard streams
# are closed, and inheritable handles
# are only inherited if the close_fds
# parameter is False."
for i in xrange(3, MAXFD):
try:
if i == errpipe_write or os.get_inheritable(i):
continue
os.close(i)
except:
pass
if restore_signals: if restore_signals:
# restore the documented signals back to sig_dfl; # restore the documented signals back to sig_dfl;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment