From 571575bb9a9657ee2cefdf8fc8bdaf894f55d830 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Mar 21 2017 12:40:36 +0000 Subject: [PATCH 1/3] Add performance repo analyzer Signed-off-by: Patrick Uiterwijk --- diff --git a/pagure/__init__.py b/pagure/__init__.py index 71a8e92..9a99089 100644 --- a/pagure/__init__.py +++ b/pagure/__init__.py @@ -38,10 +38,22 @@ from flask_multistatic import MultiStaticFlask from werkzeug.routing import BaseConverter +if os.environ.get('PAGURE_PERFREPO'): + import pagure.perfrepo as perfrepo +else: + perfrepo = None + import pagure.exceptions # Create the application. APP = MultiStaticFlask(__name__) + +if perfrepo: + # Do this as early as possible. + # We want the perfrepo before_request to be the very first thing to be run, + # so that we can properly setup the stats before the request. + APP.before_request(perfrepo.reset_stats) + APP.jinja_env.trim_blocks = True APP.jinja_env.lstrip_blocks = True @@ -783,3 +795,8 @@ if APP.config.get('PAGURE_AUTH', None) == 'local': def shutdown_session(exception=None): """ Remove the DB session at the end of each request. """ SESSION.remove() + + +if perfrepo: + # Do this at the very end, so that the after_request comes last. + APP.after_request(perfrepo.print_stats) diff --git a/pagure/perfrepo.py b/pagure/perfrepo.py new file mode 100644 index 0000000..1db4745 --- /dev/null +++ b/pagure/perfrepo.py @@ -0,0 +1,219 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2017 - Copyright Red Hat Inc + + Authors: + Patrick Uiterwijk + +""" + +import pprint +import os +import traceback +import types + +import pygit2 +import _pygit2 + + +real_pygit2_repository = pygit2.Repository + +TOTALS = {'walks': 0, + 'steps': 0} +REQUESTS = [] +STATS = {} + + +class PerfRepoMeta(type): + def __new__(cls, name, parents, dct): + # create a class_id if it's not specified + if 'class_id' not in dct: + dct['class_id'] = name.lower() + + # we need to call type.__new__ to complete the initialization + return super(PerfRepoMeta, cls).__new__(cls, name, parents, dct) + + def __getattr__(cls, attr): + real = getattr(real_pygit2_repository, attr) + if type(real).__name__ in ['function', 'builtin_function_or_method']: + def fake(*args, **kwargs): + return real(*args, **kwargs) + return fake + else: + return real + + +class FakeWalker(object): + def __init__(self, parent): + self.parent = parent + self.wid = STATS['counters']['walks'] + STATS['counters']['walks'] += 1 + + STATS['walks'][self.wid] = { + 'steps': 0, + 'type': 'walker', + 'init': traceback.extract_stack(limit=3)[0], + 'iter': None} + TOTALS['walks'] += 1 + + def __getattr__(self, attr): + return getattr(self.parent, attr) + + def __iter__(self): + STATS['walks'][self.wid]['iter'] = traceback.extract_stack(limit=2)[0] + + return self + + def next(self): + STATS['walks'][self.wid]['steps'] += 1 + TOTALS['steps'] += 1 + resp = self.parent.next() + return resp + + +class FakeDiffHunk(object): + def __init__(self, parent): + self.parent = parent + + def __getattr__(self, attr): + print 'Getting Fake Hunk %s' % attr + resp = getattr(self.parent, attr) + print 'Response: %s' % resp + return resp + + +class FakeDiffPatch(object): + def __init__(self, parent): + self.parent = parent + + def __getattr__(self, attr): + if attr == 'hunks': + return [FakeDiffHunk(h) for h in self.parent.hunks] + return getattr(self.parent, attr) + + +class FakeDiffer(object): + def __init__(self, parent): + self.parent = parent + self.iter = None + self.did = STATS['counters']['diffs'] + STATS['counters']['diffs'] += 1 + + STATS['diffs'][self.did] = { + 'init': traceback.extract_stack(limit=3)[0], + 'steps': 0, + 'iter': None} + + def __getattr__(self, attr): + return getattr(self.parent, attr) + + def __dir__(self): + return dir(self.parent) + + def __iter__(self): + STATS['diffs'][self.did]['iter'] = traceback.extract_stack(limit=2)[0] + + self.iter = self.parent.__iter__() + return self + + def next(self): + STATS['diffs'][self.did]['steps'] += 1 + resp = self.iter.next() + if isinstance(resp, _pygit2.Patch): + resp = FakeDiffPatch(resp) + else: + raise Exception('Unexpected %s returned from differ' % resp) + return resp + + def __len__(self): + return len(self.parent) + + +class PerfRepo(object): + """ An utility class allowing to go around pygit2's inability to be + stable. + + """ + __metaclass__ = PerfRepoMeta + + def __init__(self, path): + STATS['repo_inits'].append((path, traceback.extract_stack(limit=2)[0])) + STATS['counters']['inits'] += 1 + + self.repo = real_pygit2_repository(path) + self.iter = None + + def __getattr__(self, attr): + real = getattr(self.repo, attr) + if type(real) in [types.FunctionType, + types.BuiltinFunctionType, + types.BuiltinMethodType]: + def fake(*args, **kwargs): + resp = real(*args, **kwargs) + if isinstance(resp, _pygit2.Walker): + resp = FakeWalker(resp) + elif isinstance(resp, _pygit2.Diff): + resp = FakeDiffer(resp) + return resp + return fake + elif isinstance(real, dict): + real_getitem = real.__getitem__ + + def fake_getitem(self, item): + return real_getitem(item) + real.__getitem__ = fake_getitem + return real + else: + return real + + def __getitem__(self, item): + return self.repo.__getitem__(item) + + def __contains__(self, item): + return self.repo.__contains__(item) + + def __iter__(self): + self.wid = STATS['counters']['walks'] + STATS['counters']['walks'] += 1 + STATS['walks'][self.wid] = { + 'steps': 0, + 'type': 'iter', + 'iter': traceback.extract_stack(limit=3)[0]} + TOTALS['walks'] += 1 + + self.iter = self.repo.__iter__() + return self + + def next(self): + STATS['walks'][self.wid]['steps'] += 1 + TOTALS['steps'] += 1 + return self.iter.next() + +pygit2.Repository = PerfRepo + + +def reset_stats(): + """Resets STATS to be clear for the next request.""" + global STATS + STATS = {'walks': {}, + 'diffs': {}, + 'repo_inits': [], + 'counters': {'walks': 0, + 'diffs': 0, + 'inits': 0}} + +# Make sure we start blank +reset_stats() + + +def print_stats(response): + """Finalizes stats for the current request, and prints them possibly.""" + REQUESTS.append(STATS) + if not os.environ.get('PAGURE_PERFREPO_VERBOSE'): + return response + + print 'Statistics:' + pprint.pprint(STATS) + + return response diff --git a/runserver.py b/runserver.py index 44682ba..b774b75 100755 --- a/runserver.py +++ b/runserver.py @@ -23,6 +23,10 @@ parser.add_argument( default=False, help='Profile Pagure.') parser.add_argument( + '--perf-verbose', dest='perfverbose', action='store_true', + default=False, + help='Enable per-request printing of performance statistics.') +parser.add_argument( '--port', '-p', default=5000, help='Port for the Pagure to run on.') parser.add_argument( @@ -39,6 +43,10 @@ if args.config: config = os.path.join(here, config) os.environ['PAGURE_CONFIG'] = config +if args.perfverbose: + os.environ['PAGURE_PERFREPO'] = 'true' + os.environ['PAGURE_PERFREPO_VERBOSE'] = 'true' + from pagure import APP if args.profile: From fcd520a093a366ab5407f845e2b75da24f5edd28 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Mar 21 2017 12:40:36 +0000 Subject: [PATCH 2/3] Add performance checks to the test suite Signed-off-by: Patrick Uiterwijk --- diff --git a/tests/__init__.py b/tests/__init__.py index 418a01a..7267906 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -19,6 +19,9 @@ import tempfile import os logging.basicConfig(stream=sys.stderr) +# Always enable performance counting for tests +os.environ['PAGURE_PERFREPO'] = 'true' + from datetime import date from datetime import datetime from datetime import timedelta @@ -38,6 +41,7 @@ import pagure import pagure.lib import pagure.lib.model from pagure.lib.repo import PagureRepo +import pagure.perfrepo as perfrepo DB_PATH = 'sqlite:///:memory:' FAITOUT_URL = 'http://faitout.fedorainfracloud.org/' @@ -99,8 +103,32 @@ class Modeltests(unittest.TestCase): self.gitrepo = None self.gitrepos = None + def perfMaxWalks(self, max_walks, max_steps): + """ Check that we have not performed too many walks/steps. """ + num_walks = 0 + num_steps = 0 + for reqstat in perfrepo.REQUESTS: + for walk in reqstat['walks'].values(): + num_walks += 1 + num_steps += walk['steps'] + self.assertLessEqual(num_walks, max_walks, + '%s git repo walks performed, at most %s allowed' + % (num_walks, max_walks)) + self.assertLessEqual(num_steps, max_steps, + '%s git repo steps performed, at most %s allowed' + % (num_steps, max_steps)) + + def perfReset(self): + """ Reset perfrepo stats. """ + perfrepo.reset_stats() + perfrepo.REQUESTS = [] + def setUp(self): # pylint: disable=invalid-name """ Set up the environnment, ran before every tests. """ + # Clean up test performance info + perfrepo.reset_stats() + perfrepo.REQUESTS = [] + # Clean up eventual git repo left in the present folder. pagure.REDIS = None pagure.lib.REDIS = None diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index 6b43c34..dbd3bc8 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -1271,6 +1271,8 @@ class PagureFlaskRepotests(tests.Modeltests): output = self.app.get('/test') # No git repo associated self.assertEqual(output.status_code, 404) + self.perfMaxWalks(0, 0) + self.perfReset() tests.create_projects_git(self.path, bare=True) @@ -1280,6 +1282,8 @@ class PagureFlaskRepotests(tests.Modeltests): self.assertIn( '
\n' 'test project #1
', output.data) + self.perfMaxWalks(0, 0) + self.perfReset() output = self.app.get('/test/') self.assertEqual(output.status_code, 200) @@ -1287,10 +1291,13 @@ class PagureFlaskRepotests(tests.Modeltests): self.assertIn( '
\n' 'test project #1
', output.data) + self.perfMaxWalks(0, 0) + self.perfReset() # Add some content to the git repo tests.add_content_git_repo(os.path.join(self.path, 'test.git')) tests.add_readme_git_repo(os.path.join(self.path, 'test.git')) + self.perfReset() output = self.app.get('/test') self.assertEqual(output.status_code, 200) @@ -1299,6 +1306,8 @@ class PagureFlaskRepotests(tests.Modeltests): self.assertIn( '
\n' 'test project #1
', output.data) + self.perfMaxWalks(3, 8) # Target: (1, 3) + self.perfReset() # Turn that repo into a fork repo = pagure.lib.get_project(self.session, 'test') @@ -1324,6 +1333,8 @@ class PagureFlaskRepotests(tests.Modeltests): '
\n' 'test project #1
', output.data) self.assertTrue('Forked from' in output.data) + self.perfMaxWalks(1, 3) + self.perfReset() # Add a fork of a fork item = pagure.lib.model.Project( @@ -1352,6 +1363,8 @@ class PagureFlaskRepotests(tests.Modeltests): '
\n' 'test project #3
', output.data) self.assertTrue('Forked from' in output.data) + self.perfMaxWalks(3, 18) # Ideal: (1, 3) + self.perfReset() def test_view_repo_empty(self): """ Test the view_repo endpoint on a repo w/o master branch. """ From 969b905b6255070a2f539e5518baaa9888b7c516 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Mar 21 2017 12:40:36 +0000 Subject: [PATCH 3/3] Add performance totals plugin Signed-off-by: Patrick Uiterwijk --- diff --git a/nosetests b/nosetests index 2f5271b..646c4db 100755 --- a/nosetests +++ b/nosetests @@ -4,6 +4,7 @@ __requires__ = ['nose>=0.10.4', 'SQLAlchemy >= 0.7', 'jinja2 >= 2.4'] import sys from pkg_resources import load_entry_point -sys.exit( - load_entry_point('nose>=0.10.4', 'console_scripts', 'nosetests')() -) +import nose.core +from utils.perfplugin import PerfPlugin + +nose.core.main(addplugins=[PerfPlugin()]) diff --git a/runtests.sh b/runtests.sh index 3ceec59..1a84849 100755 --- a/runtests.sh +++ b/runtests.sh @@ -2,4 +2,4 @@ PAGURE_CONFIG=`pwd`/tests/test_config \ PYTHONPATH=pagure \ -./nosetests --with-coverage --cover-erase --cover-package=pagure $* +./nosetests --with-coverage --cover-erase --cover-package=pagure --with-pagureperf $* diff --git a/utils/__init__.py b/utils/__init__.py new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/utils/__init__.py diff --git a/utils/perfplugin.py b/utils/perfplugin.py new file mode 100644 index 0000000..2b270a9 --- /dev/null +++ b/utils/perfplugin.py @@ -0,0 +1,38 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2017 - Copyright Red Hat Inc + + Authors: + Patrick Uiterwijk + +""" + +import logging +import os + +from nose.plugins import Plugin + +import pagure.perfrepo as perfrepo + +log = logging.getLogger('nose.plugins.perfplugin') + + +class PerfPlugin(Plugin): + """A plugin for Nose that reports back on the test performance.""" + name = 'pagureperf' + + def options(self, parser, env=None): + if env is None: + env = os.environ + super(PerfPlugin, self).options(parser, env=env) + + def configure(self, options, conf): + super(PerfPlugin, self).configure(options, conf) + if not self.enabled: + return + + def report(self, stream): + stream.write('GIT PERFORMANCE TOTALS:\n') + stream.write('\tWalks: %d\n' % perfrepo.TOTALS['walks']) + stream.write('\tSteps: %d\n' % perfrepo.TOTALS['steps'])