From 1498ec1b2cabe63ecca089c66f0db721a4e1431f Mon Sep 17 00:00:00 2001 From: Kamil Iskra Date: Tue, 19 Dec 2017 16:18:52 -0600 Subject: [PATCH 1/4] Initial support for perf wrapper --- bin/argo-perf-wrapper | 144 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) create mode 100755 bin/argo-perf-wrapper diff --git a/bin/argo-perf-wrapper b/bin/argo-perf-wrapper new file mode 100755 index 0000000..1d9163e --- /dev/null +++ b/bin/argo-perf-wrapper @@ -0,0 +1,144 @@ +#!/usr/bin/env python2 + +from __future__ import print_function + +import argparse +import logging +import zmq +import os +import tempfile +import subprocess +import uuid + +logger = logging.getLogger('perf-wrapper') + +class PerfWrapper(object): + + """Implements middleware between the Linux perf and + the NRM downstream interface.""" + + def __init__(self): + pass + + def shutdown(self): + update = {'type': 'application', + 'event': 'exit', + 'uuid': self.app_uuid, + } + self.downstream_pub_socket.send_json(update) + + def progress_report(self, progress): + update = {'type': 'application', + 'event': 'progress', + 'payload': progress, + 'uuid': self.app_uuid, + } + self.downstream_pub_socket.send_json(update) + + def setup(self): + context = zmq.Context() + self.downstream_pub_socket = context.socket(zmq.PUB) + + downstream_pub_param = "ipc:///tmp/nrm-downstream-in" + + self.downstream_pub_socket.connect(downstream_pub_param) + + logger.info("downstream pub socket connected to: %s", + downstream_pub_param) + + # retrieve our container uuid + self.container_uuid = os.environ.get('ARGO_CONTAINER_UUID') + if self.container_uuid is None: + logger.error("missing container uuid") + exit(1) + self.app_uuid = str(uuid.uuid4()) + logger.info("client uuid: %r", self.app_uuid) + + # send an hello to the demon + update = {'type': 'application', + 'event': 'start', + 'container': self.container_uuid, + 'uuid': self.app_uuid, + 'progress': True, + 'threads': {'min': 1, 'cur': 1, 'max': 1}, + } + self.downstream_pub_socket.send_json(update) + + def main(self): + parser = argparse.ArgumentParser() + parser.add_argument("-v", "--verbose", + help="verbose logging information", + action='store_true') + parser.add_argument("-f", "--frequency", + help="sampling frequency in ms", + type=int, default=1000) + parser.add_argument("cmd", help="command and arguments", + nargs=argparse.REMAINDER) + args = parser.parse_args() + + if args.verbose: + logger.setLevel(logging.DEBUG) + + logger.info("cmd: %r", args.cmd) + + self.setup() + + # create a named pipe between us and the to-be-launched perf + # There is no mkstemp for FIFOs but we can securely create a temporary + # directory and then create a FIFO inside of it. + tmpdir = tempfile.mkdtemp() + fifoname = os.path.join(tmpdir, 'perf-fifo') + logger.info("fifoname: %r", fifoname) + os.mkfifo(fifoname, 0600) + + argv = ['perf', 'stat', '-e', 'instructions', '-x', ',', + '-I', str(args.frequency), '-o', fifoname, '--'] + argv.extend(args.cmd) + logger.info("argv: %r", argv) + + p = subprocess.Popen(argv, close_fds=True) + + # This blocks until the other end opens as well so we need to invoke + # it after Popen. + # FIXME: will deadlock if Popen fails (say, no perf). + fifo = open(fifoname, 'r') + + last_time = 0.0 + # "for line in fifo" idiom didn't work for me here -- Python was + # buffering the output internally until perf was finished. + while True: + line = fifo.readline(); + if not line: + break + + line = line.strip(); + if len(line) == 0 or line[0] == '#': + continue + tokens = line.split(',') + + logger.info("tokens: %r", tokens) + + time = float(tokens[0]) + if tokens[1] == '': + instructions = 0 + else: + instructions = int(tokens[1]) + ips = int(instructions / (time - last_time)) + + logger.info("instructions per second: %r", ips) + self.progress_report(ips) + + last_time = time + + # The child should be dead by now so this should terminate immediately. + p.wait() + + self.shutdown() + fifo.close() + os.remove(fifoname) + os.rmdir(tmpdir) + +if __name__ == "__main__": + logging.basicConfig(level=logging.WARNING) + wrapper = PerfWrapper() + wrapper.main() -- GitLab From 94bd7a1bad7416e49f017053150fb7cd8e7f6cdb Mon Sep 17 00:00:00 2001 From: Swann Perarnau Date: Tue, 19 Dec 2017 16:37:33 -0600 Subject: [PATCH 2/4] [style] Fix pep8 issues, has notified by flake8 Small things like spacing and extra semicolon. It is also recommended to use the 0o notation for octal, as a way to make them stand out in the code. --- bin/argo-perf-wrapper | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/bin/argo-perf-wrapper b/bin/argo-perf-wrapper index 1d9163e..ade92be 100755 --- a/bin/argo-perf-wrapper +++ b/bin/argo-perf-wrapper @@ -12,6 +12,7 @@ import uuid logger = logging.getLogger('perf-wrapper') + class PerfWrapper(object): """Implements middleware between the Linux perf and @@ -89,7 +90,7 @@ class PerfWrapper(object): tmpdir = tempfile.mkdtemp() fifoname = os.path.join(tmpdir, 'perf-fifo') logger.info("fifoname: %r", fifoname) - os.mkfifo(fifoname, 0600) + os.mkfifo(fifoname, 0o600) argv = ['perf', 'stat', '-e', 'instructions', '-x', ',', '-I', str(args.frequency), '-o', fifoname, '--'] @@ -107,11 +108,11 @@ class PerfWrapper(object): # "for line in fifo" idiom didn't work for me here -- Python was # buffering the output internally until perf was finished. while True: - line = fifo.readline(); + line = fifo.readline() if not line: break - line = line.strip(); + line = line.strip() if len(line) == 0 or line[0] == '#': continue tokens = line.split(',') @@ -138,6 +139,7 @@ class PerfWrapper(object): os.remove(fifoname) os.rmdir(tmpdir) + if __name__ == "__main__": logging.basicConfig(level=logging.WARNING) wrapper = PerfWrapper() -- GitLab From b666f1c2e27939ad2a7251bbcd28591915816807 Mon Sep 17 00:00:00 2001 From: Kamil Iskra Date: Tue, 19 Dec 2017 16:53:57 -0600 Subject: [PATCH 3/4] Configure perf-wrapper using the manifest --- doc/manifest | 6 ++++++ nrm/aci.py | 24 +++++++++++++++++++++++- nrm/containers.py | 5 +++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/doc/manifest b/doc/manifest index b5c6d8f..41f86b1 100644 --- a/doc/manifest +++ b/doc/manifest @@ -17,6 +17,12 @@ "cpus": "4", "mems": "1" } + }, + { + "name": "argo/perfwrapper", + "value": { + "enabled": "1" + } } ] } diff --git a/nrm/aci.py b/nrm/aci.py index f467a63..bce4eb0 100644 --- a/nrm/aci.py +++ b/nrm/aci.py @@ -125,13 +125,35 @@ class Container(SpecField): """Load container information.""" return super(Container, self).load(data) +class PerfWrapper(SpecField): + + """Information on whether to use perf for a container.""" + + fields = {"enabled": spec(unicode, False) + } + + def __init__(self): + """Create empty perf wrapper.""" + pass + + def load(self, data): + """Load perf wrapper information.""" + ret = super(PerfWrapper, self).load(data) + if not ret: + return ret + if self.enabled not in ["0", "False", "1", "True"]: + logger.error("Invalid value of perfwrapper enabled: %s", + self.enabled) + return False + return True class IsolatorList(SpecField): """Represent the list of isolator in a Manifest.""" types = {"argo/scheduler": spec(Scheduler, False), - "argo/container": spec(Container, True) + "argo/container": spec(Container, True), + "argo/perfwrapper": spec(PerfWrapper, False) } def __init__(self): diff --git a/nrm/containers.py b/nrm/containers.py index f6ad784..95028fd 100644 --- a/nrm/containers.py +++ b/nrm/containers.py @@ -65,6 +65,11 @@ class ContainerManager(object): else: argv = [] + # for now we place it within the container, but it's probably better + # if it's outside (so move it to NodeOSClient?) + if hasattr(manifest.app.isolators, 'perfwrapper') and hasattr(manifest.app.isolators.perfwrapper, 'enabled') and manifest.app.isolators.perfwrapper.enabled in ["1", "True"]: + argv.append('argo-perf-wrapper') + argv.append(command) argv.extend(args) process = self.nodeos.execute(container_name, argv, environ) -- GitLab From 41a919013f08f27859a7df2c482b81bc620445ae Mon Sep 17 00:00:00 2001 From: Kamil Iskra Date: Tue, 19 Dec 2017 18:00:17 -0600 Subject: [PATCH 4/4] Improve formatting and commentary --- nrm/containers.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/nrm/containers.py b/nrm/containers.py index 95028fd..1bba5cb 100644 --- a/nrm/containers.py +++ b/nrm/containers.py @@ -65,10 +65,14 @@ class ContainerManager(object): else: argv = [] - # for now we place it within the container, but it's probably better - # if it's outside (so move it to NodeOSClient?) - if hasattr(manifest.app.isolators, 'perfwrapper') and hasattr(manifest.app.isolators.perfwrapper, 'enabled') and manifest.app.isolators.perfwrapper.enabled in ["1", "True"]: - argv.append('argo-perf-wrapper') + # It would've been better if argo-perf-wrapper wrapped around + # argo-nodeos-config and not the final command -- that way it would + # be running outside of the container. However, because + # argo-nodeos-config is suid root, perf can't monitor it. + if hasattr(manifest.app.isolators, 'perfwrapper'): + if hasattr(manifest.app.isolators.perfwrapper, 'enabled'): + if manifest.app.isolators.perfwrapper.enabled in ["1", "True"]: + argv.append('argo-perf-wrapper') argv.append(command) argv.extend(args) -- GitLab