From 42b75b897add6ab295c18dfc4ce9937e4a212b3e Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Thu, 21 May 2009 01:44:33 +0000 Subject: - Removed the pickling of ZCML actions (the code that wrote ``configure.zcml.cache`` next to ``configure.zcml`` files in projects). The code which managed writing and reading of the cache file was a source of subtle bugs when users switched between imperative (e.g. ``@bfg_view``) registrations and declarative registrations (e.g. the ``view`` directive in ZCML) on the same project. On a moderately-sized project (535 ZCML actions and 15 ZCML files), executing actions read from the pickle was saving us only about 200ms (2.5 sec vs 2.7 sec average). On very small projects (1 ZCML file and 4 actions), startup time was comparable, and sometimes even slower when reading from the pickle, and both ways were so fast that it really just didn't matter anyway. --- CHANGES.txt | 19 ++++ docs/narr/project.rst | 13 --- docs/narr/startup.rst | 20 ++--- docs/narr/views.rst | 12 ++- repoze/bfg/registry.py | 2 +- repoze/bfg/tests/test_zcml.py | 197 +----------------------------------------- repoze/bfg/zcml.py | 90 +------------------ 7 files changed, 40 insertions(+), 313 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index d11fa5d61..817b51aff 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,22 @@ +0.8.1 (unreleased) +================== + +Features +-------- + +- Removed the pickling of ZCML actions (the code that wrote + ``configure.zcml.cache`` next to ``configure.zcml`` files in + projects). The code which managed writing and reading of the cache + file was a source of subtle bugs when users switched between + imperative (e.g. ``@bfg_view``) registrations and declarative + registrations (e.g. the ``view`` directive in ZCML) on the same + project. On a moderately-sized project (535 ZCML actions and 15 ZCML + files), executing actions read from the pickle was saving us only + about 200ms (2.5 sec vs 2.7 sec average). On very small projects (1 + ZCML file and 4 actions), startup time was comparable, and sometimes + even slower when reading from the pickle, and both ways were so fast + that it really just didn't matter anyway. + 0.8 (2009-05-18) ================ diff --git a/docs/narr/project.rst b/docs/narr/project.rst index 5b15e9f44..2bd523aab 100644 --- a/docs/narr/project.rst +++ b/docs/narr/project.rst @@ -188,19 +188,6 @@ port 6543. development easier, as changes to Python code under :mod:`repoze.bfg` is not put into effect until the server restarts. -.. note:: When :mod:`repoze.bfg` starts, it attempts to write a - ``.cache`` file which stores a cached version of your - :term:`application registry`. In a typical setup this file will be - written as ``configure.zcml.cache`` in the same directory that your - application's ``configure.zcml`` is stored. This is temporary data - that can help your :mod:`repoze.bfg` application start slightly - faster (its existence prevents the need to parse the XML stored in - the ``.zcml`` file if that file or any of files upon which it - depends files have not changed). You can delete it at will as - necessary; it will be recreated. If a ``.cache`` file cannot be - written due to filesystem permissions, :mod:`repoze.bfg` will just - reparse the ``.zcml`` file every time it starts. - Viewing the Application ----------------------- diff --git a/docs/narr/startup.rst b/docs/narr/startup.rst index 58476da79..1d8206f14 100644 --- a/docs/narr/startup.rst +++ b/docs/narr/startup.rst @@ -127,17 +127,15 @@ press ``return`` after running ``paster serve MyProject.ini``. absolute, the ``package`` argument is ignored. #. The ``make_app`` function does its work. It finds and parses the - ZCML represented by the application registry file (or may obtain - the application registry from a previously cached pickle file, - e.g. ``configure.zcml.cache``). If it fails to parse one or more - ZCML files, a ``XMLConfigurationError`` is raised (or possibly - another error if the ZCML file just doesn't exist). If it - succeeds, an :term:`application registry` is created, representing - the view registrations (and other registrations) for your - application. A :term:`router` instance is created, and the router - is associated with the application registry. The router represents - your application; the settings in the application registry that is - created will be used for your application. + ZCML represented by the application registry file. If it fails to + parse one or more ZCML files, a ``XMLConfigurationError`` is raised + (or possibly another error if the ZCML file just doesn't exist). + If it succeeds, an :term:`application registry` is created, + representing the view registrations (and other registrations) for + your application. A :term:`router` instance is created, and the + router is associated with the application registry. The router + represents your application; the settings in the application + registry that is created will be used for your application. #. A ``WSGIApplicationCreatedEvent`` event is emitted (see :ref:`events_chapter` for more informations about events). diff --git a/docs/narr/views.rst b/docs/narr/views.rst index 87f191a3e..ecaa9784b 100644 --- a/docs/narr/views.rst +++ b/docs/narr/views.rst @@ -186,13 +186,11 @@ After you do so, you will not need to use any other ZCML to configure decorator to do this work. .. warning:: using this feature tends to slows down application - startup, as the application registry is not capable of being cached - within a ``configure.zcml.cache`` file when this package is in use, - and more work is performed at application startup to scan for view - declarations. Also, if you use decorators, it means that other - people will not be able to override your view declarations - externally using ZCML: this is a common requirement if you're - developing an exensible application (e.g. a framework). + startup slightly, as more work is performed at application startup + to scan for view declarations. Also, if you use decorators, it + means that other people will not be able to override your view + declarations externally using ZCML: this is a common requirement if + you're developing an exensible application (e.g. a framework). The ``bfg_view`` Decorator ~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/repoze/bfg/registry.py b/repoze/bfg/registry.py index 5d856ca19..0cf8e306b 100644 --- a/repoze/bfg/registry.py +++ b/repoze/bfg/registry.py @@ -99,7 +99,7 @@ def populateRegistry(registry, filename, package, lock=threading.Lock()): try: original_getSiteManager.sethook(getSiteManager) zope.component.getGlobalSiteManager = registry_manager.get - zcml_configure(filename, package=package) + zcml_configure(filename, package) finally: zope.component.getGlobalSiteManager = getGlobalSiteManager lock.release() diff --git a/repoze/bfg/tests/test_zcml.py b/repoze/bfg/tests/test_zcml.py index 1853483ba..0a1103b26 100644 --- a/repoze/bfg/tests/test_zcml.py +++ b/repoze/bfg/tests/test_zcml.py @@ -453,7 +453,7 @@ class TestRoute(unittest.TestCase): self.assertEqual(route_discriminator[6], None) self.assertEqual(route_args, (route,)) -class TestZCMLPickling(unittest.TestCase): +class TestZCMLConfigure(unittest.TestCase): i = 0 def setUp(self): cleanUp() @@ -469,9 +469,6 @@ class TestZCMLPickling(unittest.TestCase): self.i += 1 self.packagepath = os.path.join(tempdir, modname) fixturedir = package_path(package) - pckname = os.path.join(fixturedir, 'configure.zcml.cache') - if os.path.isfile(pckname): - os.remove(pckname) shutil.copytree(fixturedir, self.packagepath) sys.path.insert(0, tempdir) self.module = __import__(modname) @@ -488,36 +485,10 @@ class TestZCMLPickling(unittest.TestCase): shutil.rmtree(self.tempdir) def test_file_configure(self): - import os - import cPickle from repoze.bfg.zcml import file_configure - self.assertEqual(False, file_configure('configure.zcml', self.module)) - picklename = os.path.join(self.packagepath, 'configure.zcml.cache') - self.failUnless(os.path.exists(picklename)) - actions = cPickle.load(open(picklename, 'rb')) + actions = file_configure('configure.zcml', self.module) self.failUnless(actions) - - def test_file_configure_uncacheable_removes_cache(self): - import os - from repoze.bfg.zcml import file_configure - picklename = os.path.join(self.packagepath, 'configure.zcml.cache') - f = open(picklename, 'w') - f.write('imhere') - self.failUnless(os.path.exists(picklename)) - - import repoze.bfg.zcml - keep_view = repoze.bfg.zcml.view - - def wrap_view(*arg, **kw): - kw['cacheable'] = False - return keep_view(*arg, **kw) - - try: - repoze.bfg.zcml.view = wrap_view - file_configure('configure.zcml', self.module) - self.failIf(os.path.exists(picklename)) # should be deleted - finally: - repoze.bfg.zcml.view = keep_view + self.failUnless(isinstance(actions, list)) def test_file_configure_nonexistent_configure_dot_zcml(self): import os @@ -526,168 +497,6 @@ class TestZCMLPickling(unittest.TestCase): self.assertRaises(IOError, file_configure, 'configure.zcml', self.module) - def test_file_configure_pickling_error(self): - import os - from repoze.bfg.zcml import file_configure - def dumpfail(actions, f, num): - raise IOError - self.assertEqual(False, - file_configure('configure.zcml', self.module, dumpfail)) - picklename = os.path.join(self.packagepath, 'configure.zcml.cache') - self.failIf(os.path.exists(picklename)) - - def test_zcml_configure_writes_pickle_when_none_exists(self): - import os - import cPickle - from repoze.bfg.zcml import zcml_configure - self.assertEqual(False, zcml_configure('configure.zcml', self.module)) - picklename = os.path.join(self.packagepath, 'configure.zcml.cache') - self.failUnless(os.path.exists(picklename)) - actions = cPickle.load(open(picklename, 'rb')) - self.failUnless(actions) - - def test_zcml_configure_uses_file_configure_with_bad_pickle1(self): - import os - import cPickle - from repoze.bfg.zcml import zcml_configure - picklename = os.path.join(self.packagepath, 'configure.zcml.cache') - f = open(picklename, 'wb') - cPickle.dump((), f) - f.close() - self.assertEqual(False, zcml_configure('configure.zcml', self.module)) - - def test_zcml_configure_uses_file_configure_with_bad_pickle2(self): - import os - from repoze.bfg.zcml import zcml_configure - picklename = os.path.join(self.packagepath, 'configure.zcml.cache') - f = open(picklename, 'wb') - f.write('garbage') - f.close() - self.assertEqual(False, zcml_configure('configure.zcml', self.module)) - - def test_zcml_configure_uses_file_configure_with_outofdate_pickle1(self): - import os - import cPickle - import time - from repoze.bfg.zcml import zcml_configure - basename = os.path.join(self.packagepath, 'configure.zcml') - picklename = os.path.join(self.packagepath, 'configure.zcml.cache') - self.assertEqual(False, zcml_configure('configure.zcml', self.module)) - self.failUnless(os.path.exists(picklename)) - actions = cPickle.load(open(picklename, 'rb')) - self.failUnless(actions) - os.utime(basename, (-1, time.time() + 100)) - self.assertEqual(False, zcml_configure('configure.zcml', self.module)) - - def test_zcml_configure_uses_file_configure_with_outofdate_pickle2(self): - import os - import cPickle - import time - from repoze.bfg.zcml import zcml_configure - basename = os.path.join(self.packagepath, 'another.zcml') - picklename = os.path.join(self.packagepath, 'configure.zcml.cache') - self.assertEqual(False, zcml_configure('configure.zcml', self.module)) - self.failUnless(os.path.exists(picklename)) - actions = cPickle.load(open(picklename, 'rb')) - self.failUnless(actions) - os.utime(basename, (-1, time.time() + 100)) - self.assertEqual(False, zcml_configure('configure.zcml', self.module)) - - def test_zcml_configure_uses_file_configure_with_missing_dependent(self): - import os - import cPickle - from repoze.bfg.zcml import zcml_configure - from zope.configuration.xmlconfig import ZopeXMLConfigurationError - basename = os.path.join(self.packagepath, 'another.zcml') - picklename = os.path.join(self.packagepath, 'configure.zcml.cache') - self.assertEqual(False, zcml_configure('configure.zcml', self.module)) - self.failUnless(os.path.exists(picklename)) - actions = cPickle.load(open(picklename, 'rb')) - self.failUnless(actions) - os.remove(basename) - self.assertRaises(ZopeXMLConfigurationError, zcml_configure, - 'configure.zcml', self.module) - - def test_zcml_configure_uses_file_configure_with_bad_version(self): - import os - from repoze.bfg.zcml import zcml_configure - from repoze.bfg.zcml import PVERSION - picklename = os.path.join(self.packagepath, 'configure.zcml.cache') - f = open(picklename, 'wb') - import cPickle - data = (PVERSION+1, 0, []) - cPickle.dump(data, open(picklename, 'wb')) - self.assertEqual(False, zcml_configure('configure.zcml', self.module)) - - def test_zcml_configure_uses_file_configure_with_bad_time(self): - import os - from repoze.bfg.zcml import zcml_configure - from repoze.bfg.zcml import PVERSION - picklename = os.path.join(self.packagepath, 'configure.zcml.cache') - f = open(picklename, 'wb') - import cPickle - data = (PVERSION, None, []) - cPickle.dump(data, open(picklename, 'wb')) - self.assertEqual(False, zcml_configure('configure.zcml', self.module)) - - def test_zcml_configure_uses_file_configure_with_bad_actions(self): - import os - from repoze.bfg.zcml import zcml_configure - from repoze.bfg.zcml import PVERSION - import time - picklename = os.path.join(self.packagepath, 'configure.zcml.cache') - f = open(picklename, 'wb') - import cPickle - data = (PVERSION, time.time()+500, None) - cPickle.dump(data, open(picklename, 'wb')) - self.assertEqual(False, zcml_configure('configure.zcml', self.module)) - - def test_zcml_configure_uses_file_configure_with_bad_actions2(self): - import cPickle - import os - import time - from repoze.bfg.zcml import zcml_configure - from repoze.bfg.zcml import PVERSION - picklename = os.path.join(self.packagepath, 'configure.zcml.cache') - f = open(picklename, 'wb') - actions = [(None, None, None, None, None)] - data = (PVERSION, time.time()+500, actions) - cPickle.dump(data, open(picklename, 'wb')) - self.assertEqual(False, zcml_configure('configure.zcml', self.module)) - - def test_zcml_configure_uses_good_pickle(self): - import os - import cPickle - import time - from repoze.bfg.zcml import zcml_configure - from repoze.bfg.zcml import PVERSION - basename = os.path.join(self.packagepath, 'another.zcml') - picklename = os.path.join(self.packagepath, 'configure.zcml.cache') - self.assertEqual(False, zcml_configure('configure.zcml', self.module)) - self.failUnless(os.path.exists(picklename)) - actions = cPickle.load(open(picklename, 'rb')) - self.failUnless(actions) - actions = (PVERSION, time.time()+100, actions[2]) - cPickle.dump(actions, open(picklename, 'wb')) - self.assertEqual(True, zcml_configure('configure.zcml', self.module)) - -class TestRemove(unittest.TestCase): - def _callFUT(self, name, os): - from repoze.bfg.zcml import remove - return remove(name, os) - - def test_fail(self): - class FakeOS: - def remove(self, name): - raise IOError('foo') - self.assertEqual(self._callFUT('name', FakeOS()), False) - - def test_succeed(self): - class FakeOS: - def remove(self, name): - pass - self.assertEqual(self._callFUT('name', FakeOS()), True) - class TestBFGViewFunctionGrokker(unittest.TestCase): def setUp(self): cleanUp() diff --git a/repoze/bfg/zcml.py b/repoze/bfg/zcml.py index 16e6d7012..7ea6eae8d 100644 --- a/repoze/bfg/zcml.py +++ b/repoze/bfg/zcml.py @@ -1,7 +1,3 @@ -import cPickle -import os -from os.path import realpath -import time import types from zope.configuration import xmlconfig @@ -28,8 +24,6 @@ from repoze.bfg.interfaces import IRoutesContext from repoze.bfg.interfaces import IViewPermission from repoze.bfg.interfaces import IView -from repoze.bfg.path import package_path - from repoze.bfg.security import ViewPermissionFactory import martian @@ -132,93 +126,15 @@ class IViewDirective(Interface): PVERSION = 1 -def pickle_name(name, package): - path = package_path(package) - basename = os.path.join(path, name) - return os.path.join(path, basename + '.cache') - -def zcml_configure(name, package, load=cPickle.load): - """ Execute pickled zcml actions or fall back to parsing from file - """ - pckname = pickle_name(name, package) - - if not (os.path.isfile(pckname) or os.path.islink(pckname)): - return file_configure(name, package) - - try: - vers, ptime, actions = load(open(pckname, 'rb')) - except (IOError, cPickle.UnpicklingError, EOFError, TypeError, ValueError, - AttributeError, NameError, ImportError): - return file_configure(name, package) - - if vers != PVERSION: - return file_configure(name, package) - - try: - ptime = int(ptime) - except: - return file_configure(name, package) - - if not hasattr(actions, '__iter__'): - return file_configure(name, package) - - files = set() - for action in actions: - try: - fileset = action[4] - files.update(fileset) - except (TypeError, IndexError): - return file_configure(name, package) - - for file in files: - if not(os.path.isfile(file) or os.path.islink(file)): - return file_configure(name, package) - - mtime = os.stat(realpath(file)).st_mtime - - if mtime >= ptime: - return file_configure(name, package) - - context = zope.configuration.config.ConfigurationMachine() - xmlconfig.registerCommonDirectives(context) - context.actions = actions - context.cached_execution = True - context.execute_actions() - return True - -def remove(name, os=os): # os parameterized for unit tests - try: - os.remove(name) - return True - except: - pass - return False - -def file_configure(name, package, dump=cPickle.dump): +def zcml_configure(name, package): context = zope.configuration.config.ConfigurationMachine() xmlconfig.registerCommonDirectives(context) context.package = package - xmlconfig.include(context, name, package) context.execute_actions(clear=False) + return context.actions - actions = context.actions - pckname = pickle_name(name, package) - - for action in actions: - - discriminator = action[0] - if discriminator and Uncacheable in discriminator: - remove(pckname) - return False - - try: - data = (PVERSION, time.time(), actions) - dump(data, open(pckname, 'wb'), -1) - except (OSError, IOError, TypeError, cPickle.PickleError): - remove(pckname) - - return False +file_configure = zcml_configure # backwards compat (>0.8.1) def exclude(name): if name.startswith('.'): -- cgit v1.2.3