Skip to content

Commit

Permalink
[Windows] speed up process_iter() (#2444)
Browse files Browse the repository at this point in the history
This is based on #2366 (comment). On Windows, we now determine process unique identity by using process' fast create time method. This has more chances to fail with `AccessDenied` for ADMIN owned processes, but it shouldn't matter because if we have no rights to get process ctime we'll also have no rights to accidentally `kill()` the wrong process PID anyway. This should drastically speedup `process_iter()` when used for retrieving process info one time instead of in a loop (e.g. htop like apps).
  • Loading branch information
giampaolo authored Oct 17, 2024
1 parent b1a7593 commit 567438c
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 19 deletions.
6 changes: 5 additions & 1 deletion HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ XXXX-XX-XX

**Enhancements**

- 2366_, [Windows]: drastically speedup `process_iter()`_. We now determine
process unique identity by using process "fast" create time method. This
will considerably speedup those apps which use `process_iter()`_ only once,
e.g. to look for a process with a certain name.
- 2446_: use pytest instead of unittest.
- 2448_: add ``make install-sysdeps`` target to install the necessary system
dependencies (python-dev, gcc, etc.) on all supported UNIX flavors.
Expand All @@ -25,7 +29,7 @@ XXXX-XX-XX
Python 3.13. (patch by Sam Gross)
- 2455_, [Linux]: ``IndexError`` may occur when reading /proc/pid/stat and
field 40 (blkio_ticks) is missing.
- 2457_, [AIX]: significantly improve the speed of `Process.open_files()`_ for
- 2457_, [AIX]: significantly improve the speed of `Process.open_files()`_ for
some edge cases.
- 2460_, [OpenBSD]: `Process.num_fds()`_ and `Process.open_files()`_ may fail
with `NoSuchProcess`_ for PID 0. Instead, we now return "null" values (0 and
Expand Down
47 changes: 34 additions & 13 deletions psutil/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,13 +350,13 @@ def _init(self, pid, _ignore_nsp=False):
self._last_sys_cpu_times = None
self._last_proc_cpu_times = None
self._exitcode = _SENTINEL
# cache creation time for later use in is_running() method
self._ident = (self.pid, None)
try:
self.create_time()
self._ident = self._get_ident()
except AccessDenied:
# We should never get here as AFAIK we're able to get
# process creation time on all platforms even as a
# limited user.
# This should happen on Windows only, since we use the fast
# create time method. AFAIK, on all other platforms we are
# able to get create time for all PIDs.
pass
except ZombieProcess:
# Zombies can still be queried by this class (although
Expand All @@ -368,11 +368,32 @@ def _init(self, pid, _ignore_nsp=False):
raise NoSuchProcess(pid, msg=msg)
else:
self._gone = True
# This pair is supposed to identify a Process instance
# univocally over time (the PID alone is not enough as
# it might refer to a process whose PID has been reused).
# This will be used later in __eq__() and is_running().
self._ident = (self.pid, self._create_time)

def _get_ident(self):
"""Return a (pid, uid) tuple which is supposed to identify a
Process instance univocally over time. The PID alone is not
enough, as it can be assigned to a new process after this one
terminates, so we add process creation time to the mix. We need
this in order to prevent killing the wrong process later on.
This is also known as PID reuse or PID recycling problem.
The reliability of this strategy mostly depends on
create_time() precision, which is 0.01 secs on Linux. The
assumption is that, after a process terminates, the kernel
won't reuse the same PID after such a short period of time
(0.01 secs). Technically this is inherently racy, but
practically it should be good enough.
"""
if WINDOWS:
# Use create_time() fast method in order to speedup
# `process_iter()`. This means we'll get AccessDenied for
# most ADMIN processes, but that's fine since it means
# we'll also get AccessDenied on kill().
# https://github.com/giampaolo/psutil/issues/2366#issuecomment-2381646555
self._create_time = self._proc.create_time(fast_only=True)
return (self.pid, self._create_time)
else:
return (self.pid, self.create_time())

def __str__(self):
info = collections.OrderedDict()
Expand Down Expand Up @@ -417,10 +438,10 @@ def __eq__(self, other):
# (so it has a ctime), then it turned into a zombie. It's
# important to do this because is_running() depends on
# __eq__.
pid1, ctime1 = self._ident
pid2, ctime2 = other._ident
pid1, ident1 = self._ident
pid2, ident2 = other._ident
if pid1 == pid2:
if ctime1 and not ctime2:
if ident1 and not ident2:
try:
return self.status() == STATUS_ZOMBIE
except Error:
Expand Down
4 changes: 3 additions & 1 deletion psutil/_pswindows.py
Original file line number Diff line number Diff line change
Expand Up @@ -982,14 +982,16 @@ def username(self):
return py2_strencode(domain) + '\\' + py2_strencode(user)

@wrap_exceptions
def create_time(self):
def create_time(self, fast_only=False):
# Note: proc_times() not put under oneshot() 'cause create_time()
# is already cached by the main Process class.
try:
_user, _system, created = cext.proc_times(self.pid)
return created
except OSError as err:
if is_permission_err(err):
if fast_only:
raise
debug("attempting create_time() fallback (slower)")
return self._proc_info()[pinfo_map['create_time']]
raise
Expand Down
15 changes: 12 additions & 3 deletions psutil/tests/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,22 +354,31 @@ def test_ad_on_process_creation(self):
# We are supposed to be able to instantiate Process also in case
# of zombie processes or access denied.
with mock.patch.object(
psutil.Process, 'create_time', side_effect=psutil.AccessDenied
psutil.Process, '_get_ident', side_effect=psutil.AccessDenied
) as meth:
psutil.Process()
assert meth.called

with mock.patch.object(
psutil.Process, 'create_time', side_effect=psutil.ZombieProcess(1)
psutil.Process, '_get_ident', side_effect=psutil.ZombieProcess(1)
) as meth:
psutil.Process()
assert meth.called

with mock.patch.object(
psutil.Process, 'create_time', side_effect=ValueError
psutil.Process, '_get_ident', side_effect=ValueError
) as meth:
with pytest.raises(ValueError):
psutil.Process()
assert meth.called

with mock.patch.object(
psutil.Process, '_get_ident', side_effect=psutil.NoSuchProcess(1)
) as meth:
with self.assertRaises(psutil.NoSuchProcess):
psutil.Process()
assert meth.called

def test_sanity_version_check(self):
# see: https://github.com/giampaolo/psutil/issues/564
with mock.patch(
Expand Down
3 changes: 2 additions & 1 deletion psutil/tests/test_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import signal
import socket
import stat
import string
import subprocess
import sys
import textwrap
Expand Down Expand Up @@ -813,7 +814,7 @@ def test_name(self):
@pytest.mark.skipif(PYPY or QEMU_USER, reason="unreliable on PYPY")
@pytest.mark.skipif(QEMU_USER, reason="unreliable on QEMU user")
def test_long_name(self):
pyexe = create_py_exe(self.get_testfn(suffix="0123456789" * 2))
pyexe = create_py_exe(self.get_testfn(suffix=string.digits * 2))
cmdline = [
pyexe,
"-c",
Expand Down

0 comments on commit 567438c

Please sign in to comment.