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

Improve process spawn speed and remove trailing lines. #50

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 34 additions & 33 deletions cpuset/commands/proc.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,17 @@
"""

import sys, os, io, re, logging, pwd, grp
import subprocess
from optparse import OptionParser, make_option

from cpuset import config
from cpuset import cset
from cpuset.util import *
from cpuset.commands.common import *
try: from cpuset.commands import set
try: from cpuset.commands import set
except SyntaxError:
raise
except:
except:
pass

global log
Expand All @@ -57,7 +58,7 @@
and specifying the cpuset that the process is to be created in.

For example:
# cset proc --set=blazing_cpuset --exec /usr/bin/fast_code
# cset proc --set=blazing_cpuset --exec /usr/bin/fast_code
This command will execute the /usr/bin/fast_code program
on the "blazing_cpuset" cpuset.

Expand Down Expand Up @@ -126,7 +127,7 @@
kernel threads will be added to the move. Unbound kernel threads
are those that can run on any CPU. If you also specify the
--force switch, then all tasks, kernel or not, bound or not,
will be moved.
will be moved.

CAUTION: Please be cautious with the --force switch, since moving
a kernel thread that is bound to a specific CPU to a cpuset that
Expand Down Expand Up @@ -207,14 +208,14 @@ def func(parser, options, args):

cset.rescan()

tset = None
tset = None
if options.list or options.exc:
if options.set:
tset = cset.unique_set(options.set)
elif options.toset:
tset = cset.unique_set(options.toset)
elif len(args) > 0:
if options.exc:
if options.exc:
tset = cset.unique_set(args[0])
del args[0]
else:
Expand Down Expand Up @@ -270,7 +271,7 @@ def func(parser, options, args):
set.active(tset)
# next, if there is a pidspec, move just that
if options.pid:
if options.fromset and not options.force:
if options.fromset and not options.force:
fset = cset.unique_set(options.fromset)
elif options.toset and options.set:
fset = cset.unique_set(options.set)
Expand All @@ -295,15 +296,15 @@ def func(parser, options, args):
raise CpusetException("origination cpuset not specified")
nt = len(fset.tasks)
if nt == 0:
raise CpusetException('no tasks to move from cpuset "%s"'
raise CpusetException('no tasks to move from cpuset "%s"'
% fset.path)
if options.move:
log.info('moving all tasks from %s to %s',
log.info('moving all tasks from %s to %s',
fset.name, tset.path)
selective_move(fset, tset, None, options.kthread, options.force,
options.threads)
else:
log.info('moving all kernel threads from %s to %s',
log.info('moving all kernel threads from %s to %s',
fset.path, tset.path)
# this is a -k "move", so only move kernel threads
pids = []
Expand All @@ -321,12 +322,12 @@ def list_sets(args):
log.debug("entering list_sets, args=%s", args)
l = []
if isinstance(args, list):
for s in args:
for s in args:
if isstr(s):
l.extend(cset.find_sets(s))
elif not isinstance(s, cset.CpuSet):
raise CpusetException(
'list_sets() args=%s, of which "%s" not a string or CpuSet'
'list_sets() args=%s, of which "%s" not a string or CpuSet'
% (args, s))
else:
l.append(s)
Expand All @@ -350,7 +351,7 @@ def list_sets(args):
log.info(cset.summary(s))

def move(fromset, toset, plist=None, verb=None, force=None):
log.debug('entering move, fromset=%s toset=%s list=%s force=%s verb=%s',
log.debug('entering move, fromset=%s toset=%s list=%s force=%s verb=%s',
fromset, toset, plist, force, verb)
if isstr(fromset):
fset = cset.unique_set(fromset)
Expand All @@ -368,15 +369,15 @@ def move(fromset, toset, plist=None, verb=None, force=None):
tset = toset
if plist == None:
log.debug('moving default of all processes')
if tset != fset and not force:
if tset != fset and not force:
plist = fset.tasks
else:
raise CpusetException(
"cannot move tasks into their origination cpuset")
output = 0
if verb:
if verb:
output = verb
elif verbose:
elif verbose:
output = verbose
if output:
l = []
Expand Down Expand Up @@ -410,7 +411,7 @@ def selective_move(fset, tset, plist=None, kthread=None, force=None, threads=Non
ktskb = 0
sstsk = 0
target = cset.unique_set(tset)
if fset:
if fset:
fset = cset.unique_set(fset)
if fset == target and not force:
raise CpusetException(
Expand All @@ -426,7 +427,7 @@ def selective_move(fset, tset, plist=None, kthread=None, force=None, threads=Non
# kernel threads do not have an excutable image
os.readlink('/proc/'+task+'/exe')
autsk += 1
if fset and not force:
if fset and not force:
try:
task_check.index(task)
tasks.append(task)
Expand All @@ -440,9 +441,9 @@ def selective_move(fset, tset, plist=None, kthread=None, force=None, threads=Non
if thread != task:
log.debug(' adding thread %s', thread)
tasks.append(thread)
utsk += 1
utsk += 1
except ValueError:
log.debug(' task %s not running in %s, skipped',
log.debug(' task %s not running in %s, skipped',
task, fset.name)
utsknr += 1
else:
Expand All @@ -459,11 +460,11 @@ def selective_move(fset, tset, plist=None, kthread=None, force=None, threads=Non
# this is in try because the task may not exist by the
# time we do this, in that case, just ignore it
if kthread:
if force:
if force:
tasks.append(task)
ktsk += 1
else:
if is_unbound(task):
if is_unbound(task):
tasks.append(task)
ktsk += 1
elif cset.lookup_task_from_cpusets(task) == target.path:
Expand All @@ -480,7 +481,7 @@ def selective_move(fset, tset, plist=None, kthread=None, force=None, threads=Non
ktsknr += 1
# ok, move 'em
log.debug('moving %d tasks to %s ...', len(tasks), tset.name)
if len(tasks) == 0:
if len(tasks) == 0:
log.info('**> no task matched move criteria')
if sstsk > 0:
raise CpusetException('same source/destination cpuset, use --force if ok')
Expand Down Expand Up @@ -535,8 +536,8 @@ def run(tset, args, usr_par=None, grp_par=None):
log.debug('entering run, set=%s args=%s ', s.path, args)
set.active(s)
# check user
if usr_par:
try:
if usr_par:
try:
user = pwd.getpwnam(usr_par)[2]
except KeyError:
try:
Expand All @@ -562,34 +563,34 @@ def run(tset, args, usr_par=None, grp_par=None):
pass # just forget it
# move myself into target cpuset and exec child
move_pidspec(str(os.getpid()), s)
log.info('--> last message, executed args into cpuset "%s", new pid is: %s',
s.path, os.getpid())
log.info('--> last message, executed args into cpuset "%s", new pid is: %s',
s.path, os.getpid())
# change user and group before exec
if grp_par: os.setgid(group)
if usr_par:
if usr_par:
os.setuid(user)
os.environ["LOGNAME"] = usr_par
os.environ["USERNAME"] = usr_par
os.environ["USER"] = usr_par
os.execvp(args[0], args)

def is_unbound(proc):
# FIXME: popen is slow...
# --> use /proc/<pid>/status -> Cpus_allowed
# int(line.replace(',',''), 16)
# note: delete leading zeros to compare to allcpumask
line = os.popen('/usr/bin/taskset -p ' + str(proc) +' 2>/dev/null', 'r').readline()
taskset_res = subprocess.run(['/usr/bin/taskset -p', str(proc)], capture_output=True)
line = taskset_res.stdout.readline()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this is really faster?
I think the comment suggests reading info from procfs directly instead of spawning a helper program.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would unified style help you before further modifications or what is it helpful for (it's non-functional change, right)?

Unfortunately ruff as linter shows on linting both formatting and non-formatting problems. From my experience linting catches mostly unhandled cases and type confusion.
Personally I disagree very often with what Python decided in their formatting pep(s), but configuring it feels too painful/annoying.

The subprocess substitution isn't likely speeding up it as expected.

My bad, will read from procfs directly.

aff = line.split()[-1]
log.debug('is_unbound, proc=%s aff=%s allcpumask=%s',
log.debug('is_unbound, proc=%s aff=%s allcpumask=%s',
proc, aff, cset.allcpumask)
if aff == cset.allcpumask: return True
return False

def pidspec_to_list(pidspec, fset=None, threads=False):
"""create a list of process ids out of a pidspec"""
log.debug('entering pidspecToList, pidspec=%s fset=%s threads=%s',
log.debug('entering pidspecToList, pidspec=%s fset=%s threads=%s',
pidspec, fset, threads)
if fset:
if fset:
if isstr(fset): fset = cset.unique_set(fset)
elif not isinstance(fset, cset.CpuSet):
raise CpusetException("passed fset=%s, which is not a string or CpuSet" % fset)
Expand Down
18 changes: 9 additions & 9 deletions cpuset/commands/set.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
try: from cpuset.commands import proc
except SyntaxError:
raise
except:
except:
pass

global log
Expand Down Expand Up @@ -151,7 +151,7 @@
help = 'do things recursively, use with --list and --destroy',
action = 'store_true'),
make_option('--force',
help = 'force recursive deletion even if processes are running '
help = 'force recursive deletion even if processes are running '
'in those cpusets (they will be moved to parent cpusets)',
action = 'store_true'),
make_option('-x', '--usehex',
Expand Down Expand Up @@ -254,7 +254,7 @@ def destroy_sets(sets, recurse=False, force=False):
nl.append(sets)
# check that sets passed are ok, will raise if one is bad
sl2 = []
for s in nl:
for s in nl:
st = cset.unique_set(s)
sl2.append(st)
if len(st.subsets) > 0:
Expand Down Expand Up @@ -291,7 +291,7 @@ def destroy(name):
set = cset.unique_set(name)
elif not isinstance(name, cset.CpuSet):
raise CpusetException(
"passed name=%s, which is not a string or CpuSet" % name)
"passed name=%s, which is not a string or CpuSet" % name)
else:
set = name
tsks = set.tasks
Expand Down Expand Up @@ -361,7 +361,7 @@ def create_from_options(options, args):
mspec = None
cx = None
mx = None
if options.cpu:
if options.cpu:
cset.cpuspec_check(options.cpu)
cspec = options.cpu
if options.mem:
Expand All @@ -385,11 +385,11 @@ def create(name, cpuspec, memspec, cx, mx):
try:
cset.unique_set(name)
except CpusetNotFound:
pass
pass
except:
raise CpusetException('cpuset "%s" not unique, please specify by path' % name)
else:
raise CpusetExists('attempt to create already existing set: "%s"' % name)
raise CpusetExists('attempt to create already existing set: "%s"' % name)
# FIXME: check if name is a path here
os.mkdir(cset.CpuSet.basepath+'/'+name)
# fixme: perhaps reparsing the all the sets is not so efficient...
Expand All @@ -405,7 +405,7 @@ def modify(name, cpuspec=None, memspec=None, cx=None, mx=None):
nset = cset.unique_set(name)
elif not isinstance(name, cset.CpuSet):
raise CpusetException(
"passed name=%s, which is not a string or CpuSet" % name)
"passed name=%s, which is not a string or CpuSet" % name)
else:
nset = name
log.debug('modifying cpuset "%s"', nset.name)
Expand Down Expand Up @@ -472,7 +472,7 @@ def set_details(name, indent=None, width=None, usehex=False):
if config.mread:
l.append(set.path)
l2 = []
for line in l:
for line in l:
l2.append(line.strip())
return ';'.join(l2)

Expand Down
Loading