Skip to content

Commit

Permalink
Ignore negative deltas in cpu times when calculating percentages (gia…
Browse files Browse the repository at this point in the history
  • Loading branch information
Arnon Yaari committed Feb 10, 2018
1 parent 84c2b13 commit 41cadf5
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 31 deletions.
61 changes: 30 additions & 31 deletions psutil/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1656,6 +1656,26 @@ def _cpu_busy_time(times):
return busy


def _cpu_times_deltas(t1, t2):
field_deltas = []
for field in _psplatform.scputimes._fields:
field_delta = getattr(t2, field) - getattr(t1, field)
# CPU times are always supposed to increase over time
# or at least remain the same and that's because time
# cannot go backwards.
# Surprisingly sometimes this might not be the case (at
# least on Windows and Linux), see:
# https://github.com/giampaolo/psutil/issues/392
# https://github.com/giampaolo/psutil/issues/645
# https://github.com/giampaolo/psutil/issues/1210
# Trim negative deltas to zero to ignore decreasing fields.
# top does the same. Reference:
# https://gitlab.com/procps-ng/procps/blob/v3.3.12/top/top.c#L5063
field_delta = max(0, field_delta)
field_deltas.append(field_delta)
return _psplatform.scputimes(*field_deltas)


def cpu_percent(interval=None, percpu=False):
"""Return a float representing the current system-wide CPU
utilization as a percentage.
Expand Down Expand Up @@ -1698,18 +1718,11 @@ def cpu_percent(interval=None, percpu=False):
raise ValueError("interval is not positive (got %r)" % interval)

def calculate(t1, t2):
t1_all = _cpu_tot_time(t1)
t1_busy = _cpu_busy_time(t1)
times_delta = _cpu_times_deltas(t1, t2)

t2_all = _cpu_tot_time(t2)
t2_busy = _cpu_busy_time(t2)
all_delta = _cpu_tot_time(times_delta)
busy_delta = _cpu_busy_time(times_delta)

# this usually indicates a float precision issue
if t2_busy <= t1_busy:
return 0.0

busy_delta = t2_busy - t1_busy
all_delta = t2_all - t1_all
try:
busy_perc = (busy_delta / all_delta) * 100
except ZeroDivisionError:
Expand Down Expand Up @@ -1778,28 +1791,14 @@ def cpu_times_percent(interval=None, percpu=False):

def calculate(t1, t2):
nums = []
all_delta = _cpu_tot_time(t2) - _cpu_tot_time(t1)
for field in t1._fields:
field_delta = getattr(t2, field) - getattr(t1, field)
try:
field_perc = (100 * field_delta) / all_delta
except ZeroDivisionError:
field_perc = 0.0
times_delta = _cpu_times_deltas(t1, t2)
all_delta = _cpu_tot_time(times_delta)
scale = 100.0 / max(1, all_delta)
for field_delta in times_delta:
field_perc = field_delta * scale
field_perc = round(field_perc, 1)
# CPU times are always supposed to increase over time
# or at least remain the same and that's because time
# cannot go backwards.
# Surprisingly sometimes this might not be the case (at
# least on Windows and Linux), see:
# https://github.com/giampaolo/psutil/issues/392
# https://github.com/giampaolo/psutil/issues/645
# I really don't know what to do about that except
# forcing the value to 0 or 100.
if field_perc > 100.0:
field_perc = 100.0
# `<=` because `-0.0 == 0.0` evaluates to True
elif field_perc <= 0.0:
field_perc = 0.0
# make sure we don't return negative values or values over 100%
field_perc = min(max(0.0, field_perc), 100.0)
nums.append(field_perc)
return _psplatform.scputimes(*nums)

Expand Down
46 changes: 46 additions & 0 deletions psutil/tests/test_linux.py
Original file line number Diff line number Diff line change
Expand Up @@ -1090,6 +1090,52 @@ def open_mock(name, *args, **kwargs):

self.assertEqual(psutil.PROCFS_PATH, '/proc')

def test_cpu_steal_decrease(self):
# test cumulative cpu stats decrease. We should ignore this.
# see issue #1210.
my_procfs = tempfile.mkdtemp()

# start with "steal" = 1 and everything else = 0
with open(os.path.join(my_procfs, 'stat'), 'w') as f:
f.write('cpu 0 0 0 0 0 0 0 1 0 0\n')
f.write('cpu0 0 0 0 0 0 0 0 1 0 0\n')
f.write('cpu1 0 0 0 0 0 0 0 1 0 0\n')

try:
psutil.PROCFS_PATH = my_procfs

# first call to "percent" functions should read the new stat file
# and compare to the "real" file read at import time - so the
# values are meaningless
psutil.cpu_percent()
psutil.cpu_percent(percpu=True)
psutil.cpu_times_percent()
psutil.cpu_times_percent(percpu=True)

# increase "user" while steal goes "backwards" to zero
with open(os.path.join(my_procfs, 'stat'), 'w') as f:
f.write('cpu 1 0 0 0 0 0 0 0 0 0\n')
f.write('cpu0 1 0 0 0 0 0 0 0 0 0\n')
f.write('cpu1 1 0 0 0 0 0 0 0 0 0\n')

cpu_percent = psutil.cpu_percent()
cpu_percent_percpu = psutil.cpu_percent(percpu=True)
cpu_times_percent = psutil.cpu_times_percent()
cpu_times_percent_percpu = psutil.cpu_times_percent(percpu=True)
self.assertNotEqual(cpu_percent, 0)
self.assertNotEqual(sum(cpu_percent_percpu), 0)
self.assertNotEqual(sum(cpu_times_percent), 0)
self.assertNotEqual(sum(cpu_times_percent), 100.0)
self.assertNotEqual(sum(map(sum, cpu_times_percent_percpu)), 0)
self.assertNotEqual(sum(map(sum, cpu_times_percent_percpu)), 100.0)
self.assertEqual(cpu_times_percent.steal, 0)
self.assertNotEqual(cpu_times_percent.user, 0)
finally:
shutil.rmtree(my_procfs)
reload_module(psutil)

self.assertEqual(psutil.PROCFS_PATH, '/proc')

def test_boot_time_mocked(self):
with mock.patch('psutil._pslinux.open', create=True) as m:
self.assertRaises(
Expand Down

0 comments on commit 41cadf5

Please sign in to comment.