From 1ef59f17949fa0c2faf685780bd7a097f5a67657 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Fri, 19 May 2017 17:11:52 -0500 Subject: [PATCH] Lazily populate children modules during traversal. This makes CPython quite a bit faster. --- setup.py | 3 ++ src/zope/app/apidoc/classregistry.py | 23 ++++++--- .../app/apidoc/codemodule/browser/menu.py | 4 +- src/zope/app/apidoc/codemodule/codemodule.py | 12 +++++ src/zope/app/apidoc/codemodule/configure.zcml | 4 ++ src/zope/app/apidoc/codemodule/module.py | 51 +++++++++++++------ src/zope/app/apidoc/ifacemodule/browser.py | 25 ++++----- src/zope/app/apidoc/tests.py | 2 + 8 files changed, 87 insertions(+), 37 deletions(-) diff --git a/setup.py b/setup.py index 61230b75..54d6c2dc 100644 --- a/setup.py +++ b/setup.py @@ -42,6 +42,9 @@ def read(*rnames): 'zope.testing', 'zope.testrunner', + # If we wanted to depend on lxml, it can parse the results quite + # a bit faster. + # Things we don't use or configure, but which are # picked up indirectly by other packages and # need to be loaded to avoid errors running the diff --git a/src/zope/app/apidoc/classregistry.py b/src/zope/app/apidoc/classregistry.py index 485ede85..99205d38 100644 --- a/src/zope/app/apidoc/classregistry.py +++ b/src/zope/app/apidoc/classregistry.py @@ -21,28 +21,39 @@ # TODO: List hard-coded for now. IGNORE_MODULES = ['twisted'] +import operator import sys +from six import iteritems + +_pathgetter = operator.itemgetter(0) class ClassRegistry(dict): """A simple registry for classes.""" + # This is not a WeakValueDictionary; the classes in here + # are kept alive almost certainly by the codemodule.class_.Class object, + # which in turn is kept alive by a codemodule.module.Module chain going + # all the way back to the APIDocumentation object registered with the + # global site manager. So they can't go away without clearing all that, + # which happens (usually only) with test tear downs. + def getClassesThatImplement(self, iface): """Return all class items that implement iface. Methods returns a list of 2-tuples of the form (path, class). """ - result = [(path, klass) for path, klass in self.items() - if iface.implementedBy(klass)] - result.sort(key=lambda x: x[0]) - return result + return sorted(((path, klass) for path, klass in iteritems(self) + if iface.implementedBy(klass)), + key=_pathgetter) def getSubclassesOf(self, klass): """Return all class items that are proper subclasses of klass. Methods returns a list of 2-tuples of the form (path, class). """ - return [(path, klass2) for path, klass2 in self.items() - if issubclass(klass2, klass) and klass2 is not klass] + return sorted(((path, klass2) for path, klass2 in iteritems(self) + if issubclass(klass2, klass) and klass2 is not klass), + key=_pathgetter) classRegistry = ClassRegistry() diff --git a/src/zope/app/apidoc/codemodule/browser/menu.py b/src/zope/app/apidoc/codemodule/browser/menu.py index 8ef97b69..e4da7285 100644 --- a/src/zope/app/apidoc/codemodule/browser/menu.py +++ b/src/zope/app/apidoc/codemodule/browser/menu.py @@ -99,8 +99,8 @@ def findAllClasses(self): Make sure we're registered. - >>> traverse(menu.context, 'zope/app/apidoc/codemodule/browser/menu') - + >>> traverse(menu.context, 'zope/app/apidoc/codemodule/browser/menu/Menu') + Testing the method with various inputs. diff --git a/src/zope/app/apidoc/codemodule/codemodule.py b/src/zope/app/apidoc/codemodule/codemodule.py index 3253dc25..28480b97 100644 --- a/src/zope/app/apidoc/codemodule/codemodule.py +++ b/src/zope/app/apidoc/codemodule/codemodule.py @@ -103,9 +103,21 @@ def isPackage(self): def get(self, key, default=None): """See zope.container.interfaces.IReadContainer.""" self.setup() + # TODO: Do we really like that this allows importing things from + # outside our defined namespace? This can lead to a static + # export with unreachable objects (not in the menu) return super(CodeModule, self).get(key, default) def items(self): """See zope.container.interfaces.IReadContainer.""" self.setup() return super(CodeModule, self).items() + +def _cleanUp(): + from zope.component import getGlobalSiteManager + code = getGlobalSiteManager().queryUtility(IDocumentationModule, name='Code') + if code is not None: + code.__init__() + +from zope.testing.cleanup import addCleanUp +addCleanUp(_cleanUp) diff --git a/src/zope/app/apidoc/codemodule/configure.zcml b/src/zope/app/apidoc/codemodule/configure.zcml index 5d0d9bc6..6f3b9c84 100644 --- a/src/zope/app/apidoc/codemodule/configure.zcml +++ b/src/zope/app/apidoc/codemodule/configure.zcml @@ -7,6 +7,10 @@ + + + + diff --git a/src/zope/app/apidoc/codemodule/module.py b/src/zope/app/apidoc/codemodule/module.py index bbb2030d..c329a7fc 100644 --- a/src/zope/app/apidoc/codemodule/module.py +++ b/src/zope/app/apidoc/codemodule/module.py @@ -20,6 +20,7 @@ import six import zope +from zope.cachedescriptors.property import Lazy from zope.proxy import getProxiedObject from zope.interface import implementer from zope.interface import providedBy @@ -200,19 +201,13 @@ def __setup(self): zope.deprecation.__show__.on() def withParentAndName(self, parent, name): - located = type(self)(parent, name, self._module, False) - new_children = located._children - for x in self._children.values(): - try: - new_child = x.withParentAndName(located, x.__name__) - except AttributeError: - if isinstance(x, LocationProxy): - new_child = LocationProxy(getProxiedObject(x), located, x.__name__) - else: - new_child = LocationProxy(x, located, x.__name__) - - new_children[x.__name__] = new_child - + located = _LazyModule(self, parent, name, self._module) + # Our module tree can be very large, but typically during any one + # traversal we're only going to need one specific branch. So + # initializing it lazily the first time one specific level's _children + # is accessed has a *major* performance benefit. + # A plain @Lazy attribute won't work since we need to copy from self; + # we use a subclass, with the provisio that it can be the *only* subclass return located def getDocString(self): @@ -250,11 +245,13 @@ def get(self, key, default=None): if obj is not None: child = Module(self, key, obj) - self._children[key] = child + # But note that we don't hold on to it. This is a transient + # object, almost certainly not actually in our namespace. + # TODO: Why do we even allow this? It leads to much larger static exports + # and things that aren't even reachable from the menus. return child # Maybe it is a simple attribute of the module - assert obj is None obj = getattr(self._module, key, default) if obj is not default: obj = LocationProxy(obj, self, key) @@ -268,3 +265,27 @@ def items(self): return [(name, value) for name, value in self._children.items() if not name.startswith('_')] + +class _LazyModule(Module): + + copy_from = None + + def __init__(self, copy_from, parent, name, module): + Module.__init__(self, parent, name, module, False) + del self._children # get our @Lazy back + self._copy_from = copy_from + + @Lazy + def _children(self): + new_children = {} + for x in self._copy_from._children.values(): + try: + new_child = x.withParentAndName(self, x.__name__) + except AttributeError: + if isinstance(x, LocationProxy): + new_child = LocationProxy(getProxiedObject(x), self, x.__name__) + else: + new_child = LocationProxy(x, self, x.__name__) + + new_children[x.__name__] = new_child + return new_children diff --git a/src/zope/app/apidoc/ifacemodule/browser.py b/src/zope/app/apidoc/ifacemodule/browser.py index 9ecc9b64..14f68bc7 100644 --- a/src/zope/app/apidoc/ifacemodule/browser.py +++ b/src/zope/app/apidoc/ifacemodule/browser.py @@ -13,30 +13,29 @@ ############################################################################## """Interface Details View -$Id$ """ __docformat__ = 'restructuredtext' import inspect -from zope.component import getUtility from zope.i18nmessageid import ZopeMessageFactory as _ +from zope.location.interfaces import LocationError from zope.publisher.interfaces.browser import IBrowserRequest from zope.publisher.interfaces.xmlrpc import IXMLRPCRequest from zope.publisher.interfaces.http import IHTTPRequest from zope.publisher.interfaces.ftp import IFTPRequest from zope.publisher.browser import BrowserView -from zope.security.proxy import isinstance, removeSecurityProxy +from zope.security.proxy import removeSecurityProxy from zope.proxy import removeAllProxies -from zope.traversing.api import getName, getParent, traverse +from zope.traversing.api import getName, traverse from zope.traversing.browser import absoluteURL from zope.app.apidoc.utilities import getPythonPath, renderText -from zope.app.apidoc.apidoc import APIDocumentation from zope.app.apidoc import classregistry from zope.app.apidoc import interface, component, presentation from zope.app.apidoc.browser.utilities import findAPIDocumentationRootURL +from zope.app.apidoc.browser.utilities import findAPIDocumentationRoot class InterfaceDetails(BrowserView): """View class for an Interface.""" @@ -240,9 +239,6 @@ def _prepareViews(self): views[(type in views) and type or None].append(reg) - - sort_function = lambda x, y: cmp(x['name'], y['name']) - for type, sel_views in views.items(): for level, qualifier in level_map.items(): regs = tuple(component.filterAdapterRegistrations( @@ -272,19 +268,20 @@ def getViewTypeTitles(self): class InterfaceBreadCrumbs(object): """View that provides breadcrumbs for interface objects""" + context = None + request = None + def __call__(self): """Create breadcrumbs for an interface object. The breadcrumbs are rooted at the code browser. """ - docroot = self.context - while not isinstance(docroot, APIDocumentation): - docroot = getParent(docroot) + docroot = findAPIDocumentationRoot(self.context) codeModule = traverse(docroot, "Code") crumbs = [{ 'name': _('[top]'), 'url': absoluteURL(codeModule, self.request) - }] + }] # We need the __module__ of the interface, not of a location proxy, # so we have to remove all proxies. iface = removeAllProxies(self.context) @@ -295,9 +292,9 @@ def __call__(self): crumbs.append({ 'name': name, 'url': absoluteURL(obj, self.request) - }) + }) crumbs.append({ 'name': iface.__name__, 'url': absoluteURL(self.context, self.request) - }) + }) return crumbs diff --git a/src/zope/app/apidoc/tests.py b/src/zope/app/apidoc/tests.py index fe403f55..7a4c692d 100644 --- a/src/zope/app/apidoc/tests.py +++ b/src/zope/app/apidoc/tests.py @@ -74,6 +74,8 @@ def _setUp_LayerPlace(test): test.globs['apidoc'] = APIDocumentation(root_folder, '++apidoc++') test.globs['rootFolder'] = root_folder + from zope.app.apidoc.codemodule import codemodule + codemodule._cleanUp() def _tearDown_LayerPlace(test): _tearDown_AppSetup()