4
\$\begingroup\$

We don't have the possibility to install modules like psutil on some boxes, thus I decided to write a simple script to calculate the CPU percentage by the given PID.

It uses /proc/stat to get CPU times and /proc/<pid>/stat to get the process CPU usage.

I added top output here, just to compare results and to be sure it's (about) correct. But - I don't like to use external utilities in Python scripts, thus top will be removed.

  • OS: CentOS 6.5
  • Python: 2.6.6

Next - it calculates % of CPU, taken by this process:

#!/usr/bin/env python

import sys
import os
import time
import subprocess

if len(sys.argv) == 2:
    pid = sys.argv[1]
else:
    print('No PID specified. Usage: %s <PID>' % os.path.basename(__file__))
    sys.exit(1)


def proct(pid):
    try:
        with open(os.path.join('/proc/', pid, 'stat'), 'r') as pidfile:
            proctimes = pidfile.readline()
            # get utime from /proc/<pid>/stat, 14 item
            utime = proctimes.split(' ')[13]
            # get stime from proc/<pid>/stat, 15 item
            stime = proctimes.split(' ')[14]
            # count total process used time
            proctotal = int(utime) + int(stime)
            return(float(proctotal))
    except IOError as e:
        print('ERROR: %s' % e)
        sys.exit(2)


def cput():
    try:
        with open('/proc/stat', 'r') as procfile:
            cputimes = procfile.readline()
            cputotal = 0
            # count from /proc/stat: user, nice, system, idle, iowait, irc, softirq, steal, guest
            for i in cputimes.split(' ')[2:]:
                i = int(i)
                cputotal = (cputotal + i)
            return(float(cputotal))
    except IOError as e:
        print('ERROR: %s' % e)
        sys.exit(3)

# assign start values before loop them
proctotal = proct(pid)
cputotal = cput()

try:
    while True:

        # for test, to compare results
        proc = subprocess.Popen("top -p %s -b -n 1 | grep -w mysql | awk '{print $9}'" % pid, shell=True, stdout=subprocess.PIPE)
        cpu_percentage = proc.communicate()
        print('With TOP: %s' % (cpu_percentage[0].rstrip('\n')))

        pr_proctotal = proctotal
        pr_cputotal = cputotal

        proctotal = proct(pid)
        cputotal = cput()

        try:
            res = ((proctotal - pr_proctotal) / (cputotal - pr_cputotal) * 100)
            print('With script: %s\n' % round(res, 1))
        except ZeroDivisionError:
            pass

        time.sleep(1)
except KeyboardInterrupt:
    sys.exit(0)

And its results:

$ ./cpu_usage_by_proccess.py 24846
With TOP: 0.0
With script: 0.0

With TOP: 0.0
With script: 0.0

With TOP: 0.0
With script: 0.7

With TOP: 14.0
With script: 4.8

With TOP: 32.0
With script: 34.4

With TOP: 29.9
With script: 32.6

With TOP: 30.0
With script: 35.1

With TOP: 18.0
With script: 26.5

With TOP: 20.0
With script: 20.6

What's done incorrectly here, or what can be done better (maybe a more "Pythonic way")?

\$\endgroup\$
5
\$\begingroup\$

You have code outside of functions or an if __name__ == "__main__" guard (which isn't ideal in itself), both above and below the various function definitions, which makes the code relatively difficult to follow. I would structure the script as follows:

import os
import subprocess
import sys
import time

def proct(pid):
    ...

def cput():
    ...

def main(pid):
    ...

def parse_args():
    ...
    return pid

if __name__ == "__main__":
    main(parse_args())

Now if you want to import this functionality elsewhere, it's easy. Also, note that I have made the imports alphabetical, per the style guide.


I would factor out any temporary variables, e.g.

proctotal = int(utime) + int(stime)
return(float(proctotal))

can be written:

return float(int(utime) + int(stime))

Try not to do the same thing twice, e.g.

proctimes = pidfile.readline()
utime = proctimes.split(' ')[13]
stime = proctimes.split(' ')[14]

could be rewritten

proctimes = pidfile.readline().split(' ')
utime = proctime[13] # you could also call int here
stime = proctime[14]

which only calls str.split once.


Consider adding docstrings to your functions, rather than the inline comments (some of which are redundant - utime = proctimes.split(' ')[13] does not need the explanation # get utime from /proc/<pid>/stat, 14 item, which just adds an extra line to read). Better function names would also help with clarity - e.g. cpu_total rather than cput.


The main part of cput:

cputimes = procfile.readline()
cputotal = 0
for i in cputimes.split(' ')[2:]:
    i = int(i)
    cputotal = (cputotal + i)
return(float(cputotal))

can be rewritten in a functional style:

return sum(map(float, procfile.readline().split(' ')[2:]))

or using a generator expression:

return sum(float(s) for s in procfile.readline().split(' ')[2:])
\$\endgroup\$
  • \$\begingroup\$ Idon't familiar with sum and map yet, thus - this part will left as it is, for now... All other - thanks. Moving everything to functions looks much better (usually I do so with '''Docstrings''' there, just... little bit lazy today :-)). \$\endgroup\$ – setevoy Oct 25 '14 at 12:56
4
\$\begingroup\$

I can't comment on the correctness but here's an improved version of your cput function. I'll start with the final version and then work towards it from the original code. Note the bug fix: [2:] should be [1:].

def read_cpu_usage(stat_path='/proc/stat'):
    with open(stat_path) as stat_file:
        return sum(float(time) for time in next(stat_file).split()[1:])

Remove the try/catch exit. Let Python crash loudly so we get the whole trackback

def cput():
    with open('/proc/stat', 'r') as procfile:
        cputimes = procfile.readline()
        cputotal = 0
        # count from /proc/stat: user, nice, system, idle, iowait, irc, softirq, steal, guest
        for i in cputimes.split(' ')[1:]:
            i = int(i)
            cputotal = (cputotal + i)
        return(float(cputotal))

Use the built-in sum to perform the summation.

def cput():
    with open('/proc/stat', 'r') as procfile:
        cputimes = procfile.readline()
        cputotal = sum(int(i) for i in cputimes.split(' ')[1:])
        return(float(cputotal))

Parse directly to float so we don't need to do two different conversions.

def cput():
    with open('/proc/stat', 'r') as procfile:
        cputimes = procfile.readline()
        cputotal = sum(float(i) for i in cputimes.split(' ')[1:])
        return(cputotal)

Return the total directly removing the temporary variable.

def cput():
    with open('/proc/stat', 'r') as procfile:
        cputimes = procfile.readline()
        return sum(float(i) for i in cputimes.split(' ')[1:])

Split at white-space, rather an a single space.

def cput():
    with open('/proc/stat', 'r') as procfile:
        cputimes = procfile.readline()
        return sum(float(i) for i in cputimes.split()[1:])

Default mode is read and normally implicit.

def cput():
    with open('/proc/stat') as procfile:
        cputimes = procfile.readline()
        return sum(float(i) for i in cputimes.split()[1:])

Take advantage that File are generators and remove temporary variable.

def cput():
    with open('/proc/stat') as procfile:
        return sum(float(i) for i in next(procfile).split()[1:])

(Possibly) better names.

def read_cpu_usage():
    with open('/proc/stat') as stat_file:
        return sum(float(time) for time in next(stat_file).split()[1:])

Make the path a parameter so that it can be swapped out testing with a mock file.

def read_cpu_usage(stat_path='/proc/stat'):
    with open(stat_path) as stat_file:
        return sum(float(time) for time in next(stat_file).split()[1:])

Bonus: Avoid the list copy

I'd use this version my code base but only because I know my team (i.e., one other person I've working with since we started university together in 2008) is comfortable with iterators and generators. The performance/memory gain is negligible but, for me, it the principle and consistency. Note: I've forgone PEP8's double line separating for space, but include in your code base.

from itertools import islice

def read_cpu_usage(stat_path='/proc/stat'):
    with open(stat_path) as stat_file:
        return sum(float(time) for time in
                islice(next(stat_file).split(), 1, None))

Bonus: Close the file fast

If the code is run in a much larger, concurrent program. Closing files fast helps keep away from your file descriptor limit.

from itertools import islice

def read_cpu_usage(stat_path='/proc/stat'):
    with open(stat_path) as stat_file:
        cpu_stat_line = next(stat_file)
    return sum(float(time) for time in 
            islice(cpu_stat_line.split(), 1, None))

Bonus: Split the read and parse

This makes it easier to test and, more importantly, reuse.

from itertools import islice

def read_cpu_usage(stat_path='/proc/stat'):
    with open(stat_path) as stat_file:
        return next(stat_file)

def parse_cpu_usage(cpu_stat_line):
    return sum(float(time) for time in
            islice(cpu_stat_line.split(), 1, None))

Bonus: Provide a convenience function

from itertools import islice

def get_cpu_usage(*args, **kwargs):
    return parse_cpu_usage(read_cpu_usage(*args, **kwargs))

def read_cpu_usage(stat_path='/proc/stat'):
    with open(stat_path) as stat_file:
        return next(stat_file)

def parse_cpu_usage(cpu_stat_line):
    return sum(float(time) for time in
            islice(cpu_stat_line.split(), 1, None))    

Bouns: Group the functionality into class

from itertools import islice

class CPUUsage(object):
    def __init__(self, stat_path='/proc/stat'):
        self.stat_path = stat_path

    def get(self):
        return self.parse(self.read(self.stat_path))
    __call__ = get

    @staticmethod
    def read(stat_path):
        with open(stat_path) as stat_file:
            return next(stat_file)

    @staticmethod
    def parse(cpu_stat_line):
        return sum(float(time) for time in
                islice(cpu_stat_line.split(), 1, None))

Bonus: Support the File API

Allow read from anything that implements the File API.

from itertools import islice

class CPUUsage(object):
    def __init__(self, stat_path='/proc/stat'):
        self.stat_path = stat_path

    def get(self):
        return self.parse(self.read(self.stat_path))
    __call__ = get

    @staticmethod
    def read(stat_source):
        if isinstance(stat_source, basestring):
            open_stat = lambda: open(stat_source)
        else:
            open_stat = lambda: stat_source
        with open_stat() as stat_file:
            return next(stat_file)

    @staticmethod
    def parse(cpu_stat_line):
        return sum(float(time) for time in
                islice(cpu_stat_line.split(), 1, None))

Bonus: Put it in it's own module...

Put it on the Python path, never write the code again. Also provide a convenience module-level function for the 99% use case.

from itertools import islice

__all__ = ['CPUUsage', 'get_cpu_usage']

_instance = None

def _get_instance():
    global _instance
    if _instance is None:
        _instance = CPUUsage()
    return _instance

def get_cpu_usage():
    return _get_instance().get()

class CPUUsage(object):
    def __init__(self, stat_path='/proc/stat'):
        self.stat_path = stat_path

    def get(self):
        return self.parse(self.read(self.stat_path))
    __call__ = get

    @staticmethod
    def read(stat_source):
        if isinstance(stat_source, basestring):
            open_stat = lambda: open(stat_source)
        else:
            open_stat = lambda: stat_source
        with open_stat() as stat_file:
            return next(stat_file)

    @staticmethod
    def parse(cpu_stat_line):
        return sum(float(time) for time in
                islice(cpu_stat_line.split(), 1, None))
\$\endgroup\$
3
\$\begingroup\$

@jonrsharpe has listed a number of presentation, structural, and functional changes, but I'm just going to run through some of the bugs... ;-)


TOP takes a second

Your basic program structure is:

proctotal = proct(pid)
cputotal = cput()

try:
    while True:

        # for test, to compare results
        proc = subprocess.Popen("top -p %s -b -n 1 | grep -w mysql | awk '{print $9}'" % pid, shell=True, stdout=subprocess.PIPE)
        cpu_percentage = proc.communicate()
        print('With TOP: %s' % (cpu_percentage[0].rstrip('\n')))

        pr_proctotal = proctotal
        pr_cputotal = cputotal

        proctotal = proct(pid)
        cputotal = cput()

        try:
            res = ((proctotal - pr_proctotal) / (cputotal - pr_cputotal) * 100)
            print('With script: %s\n' % round(res, 1))
        except ZeroDivisionError:
            pass

        time.sleep(1)

So, what does that do? It takes a reading from the stat files, then it runs top, which taks a second, then it re-reads the stat files, and prints the results for that second to match against TOP. Finally it waits a second, then loops.

On the next loop, it has already waited a second, then another second for the top, so the subsequent loops report the top times for only the second second of a 2-second wait time. Your comparison numbers will never match accurately.

Additionally, when you remove the top comparison, your loop will be:

proctotal = proct(pid)
cputotal = cput()

try:
    while True:

        pr_proctotal = proctotal
        pr_cputotal = cputotal

        proctotal = proct(pid)
        cputotal = cput()

        try:
            res = ((proctotal - pr_proctotal) / (cputotal - pr_cputotal) * 100)
            print('With script: %s\n' % round(res, 1))
        except ZeroDivisionError:
            pass

        time.sleep(1)

and the first time through the loop it will always display 0 because it never waited between readings. You should put the sleep(1) as the first thing inside the loop, or, while you are using top as a test, you should comment out the sleep(1) because the top call essentially is a sleep(1).

/proc/stat

Your processing of the stat files is broken.

The format for the file is (taken from my machine):

abox:~> cat /proc/stat
cpu  19407338 17453351 34946083 2361383858 59819619 2748 2690630 0 0 0
cpu0 8423923 911989 15385729 250602858 21892989 2614 2574046 0 0 0
cpu1 1057945 1098915 1028942 309149453 2009689 2 7469 0 0 0
cpu2 3439928 4831311 5750843 283795300 13861225 74 42971 0 0 0
.....

You read the first line and process that. That's the right thing to do, but, what you do is:

        cputotal = 0
        # count from /proc/stat: user, nice, system, idle, iowait, irc, softirq, steal, guest
        for i in cputimes.split(' ')[2:]:
            i = int(i)
            cputotal = (cputotal + i)
        return(float(cputotal))

The best documentation for that file is here(http://www.mjmwired.net):

  • user: normal processes executing in user mode
  • nice: niced processes executing in user mode
  • system: processes executing in kernel mode
  • idle: twiddling thumbs
  • iowait: waiting for I/O to complete
  • irq: servicing interrupts
  • softirq: servicing softirqs
  • steal: involuntary wait
  • guest: running a normal guest
  • guest_nice: running a niced guest

Now, the first line is

     cpu  19407338 17453351 34946083 2361383858 59819619 2748 2690630 0 0 0

You use the array slice [2:], which had me confused for a long time. TO me this was just counting the nice times and on, and not counting the user time at all. I never saw tha tyou were splitting on ' ' instead of just whitespace, so. Making your code depend on the amount of whitespace is a problem, and will become a bug at some future time, and, it appears, is a problem right now. You should split on whitespace, and count fields, instead of counting spaces....

Now, the last two columns are 'guest' time, and these values are double-counted in the user times. Your current code double-counts that time, and will lead to errors in your computations.

Your array slice should be splitting on whitespace, and should only be summing: [1:9]

/proc/{pid}/stat

Your calculation here is right, but I would do the split just once, and read the two fields in one slice.

\$\endgroup\$
  • \$\begingroup\$ > Your totals are off // well... if take a look here: stackoverflow.com/questions/23367857/… "According the htop source code" - need to calculate total spent CPU time, including idle, irc etc. And it sounds logical. \$\endgroup\$ – setevoy Oct 26 '14 at 5:12
  • \$\begingroup\$ @setevoy - My solution to your bug is wrong, but you still have a bug... a significant one. You are not counting the user time at all, just the nice time. \$\endgroup\$ – rolfl Oct 26 '14 at 14:16
  • \$\begingroup\$ Note, you should be slicing stat line [1:] at least, not [2:]. \$\endgroup\$ – rolfl Oct 26 '14 at 14:18
  • \$\begingroup\$ Arghhh, you split on ' ' and not \s... horrible, horrible. comment it at least. \$\endgroup\$ – rolfl Oct 26 '14 at 14:20
2
\$\begingroup\$

Not sure how relevant this is, but I saw one small issue in the original script (possibly elsewhere):

In the comparison with top, grep is used to filter out results, but "hardcoded" to mysql, which doesn't seem to be generic... i.e. proc = subprocess.Popen("top -p %s -b -n 1 | grep -w mysql | awk '{print $9}'" % pid, shell=True, stdout=subprocess.PIPE)

I'd suggest replacing "mysql" with the process name that is linked to the PID in question.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.