[optimization] improve massive write performance by optimizing hooks selection stable
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Wed, 31 Mar 2010 09:55:19 +0200
branchstable
changeset 5093 8d073d2e089d
parent 5092 e126becc1263
child 5094 13b7f30db0bb
[optimization] improve massive write performance by optimizing hooks selection profiling on some massive deletion showed up that 2/3 of the time was spent in hooks selection. Those changes make it much more acceptable (through selection is still not negligeable): * use one registry for each event, so we've much less hooks to check when emiting an event as well as no more need for the match_event selector. This required ability to put one appobject into several registries, using a __registries__ class attribute. * check for deprecated .enabled at registry initialization time instead of at selection time A very simple HooksManager class has been reintroduce to choose the right registry on call_hooks. Those optimisations leads to a ~x3 factor of time necessary to delete 16000 entities.
appobject.py
server/hook.py
server/repository.py
server/test/unittest_hook.py
vregistry.py
--- a/appobject.py	Wed Mar 31 09:45:14 2010 +0200
+++ b/appobject.py	Wed Mar 31 09:55:19 2010 +0200
@@ -14,6 +14,7 @@
 from warnings import warn
 
 from logilab.common.deprecation import deprecated
+from logilab.common.decorators import classproperty
 from logilab.common.logging_ext import set_log_methods
 
 
@@ -245,6 +246,12 @@
     __regid__ = None
     __select__ = yes()
 
+    @classproperty
+    def __registries__(cls):
+        if cls.__registry__ is None:
+            return ()
+        return (cls.__registry__,)
+
     @classmethod
     def __registered__(cls, registry):
         """called by the registry when the appobject has been registered.
--- a/server/hook.py	Wed Mar 31 09:45:14 2010 +0200
+++ b/server/hook.py	Wed Mar 31 09:55:19 2010 +0200
@@ -48,6 +48,7 @@
 from logilab.common.deprecation import deprecated
 from logilab.common.logging_ext import set_log_methods
 
+from cubicweb import RegistryNotFound
 from cubicweb.cwvreg import CWRegistry, VRegistry
 from cubicweb.selectors import (objectify_selector, lltrace, ExpectedValueSelector,
                                 implements)
@@ -66,6 +67,12 @@
 
 
 class HooksRegistry(CWRegistry):
+    def initialization_completed(self):
+        for appobjects in self.values():
+            for cls in appobjects:
+                if not cls.enabled:
+                    warn('[3.6] %s: enabled is deprecated' % cls)
+                    self.unregister(cls)
 
     def register(self, obj, **kwargs):
         try:
@@ -96,7 +103,19 @@
                     for hook in hooks:
                         hook()
 
-VRegistry.REGISTRY_FACTORY['hooks'] = HooksRegistry
+class HooksManager(object):
+    def __init__(self, vreg):
+        self.vreg = vreg
+
+    def call_hooks(self, event, session=None, **kwargs):
+        try:
+            self.vreg['%s_hooks' % event].call_hooks(event, session, **kwargs)
+        except RegistryNotFound:
+            pass # no hooks for this event
+
+
+for event in ALL_HOOKS:
+    VRegistry.REGISTRY_FACTORY['%s_hooks' % event] = HooksRegistry
 
 _MARKER = object()
 def entity_oldnewvalue(entity, attr):
@@ -116,21 +135,6 @@
 
 @objectify_selector
 @lltrace
-def _bw_is_enabled(cls, req, **kwargs):
-    if cls.enabled:
-        return 1
-    warn('[3.6] %s: enabled is deprecated' % cls)
-    return 0
-
-@objectify_selector
-@lltrace
-def match_event(cls, req, **kwargs):
-    if kwargs.get('event') in cls.events:
-        return 1
-    return 0
-
-@objectify_selector
-@lltrace
 def enabled_category(cls, req, **kwargs):
     if req is None:
         return True # XXX how to deactivate server startup / shutdown event
@@ -190,11 +194,11 @@
                 return 1
         return 0
 
+
 # base class for hook ##########################################################
 
 class Hook(AppObject):
-    __registry__ = 'hooks'
-    __select__ = match_event() & enabled_category() & _bw_is_enabled()
+    __select__ = enabled_category()
     # set this in derivated classes
     events = None
     category = None
@@ -203,6 +207,10 @@
     enabled = True
 
     @classproperty
+    def __registries__(self):
+        return ['%s_hooks' % ev for ev in self.events]
+
+    @classproperty
     def __regid__(cls):
         warn('[3.6] %s.%s: please specify an id for your hook'
              % (cls.__module__, cls.__name__), DeprecationWarning)
--- a/server/repository.py	Wed Mar 31 09:45:14 2010 +0200
+++ b/server/repository.py	Wed Mar 31 09:55:19 2010 +0200
@@ -206,7 +206,7 @@
         self._shutting_down = False
         if config.quick_start:
             config.init_cubes(self.get_cubes())
-        self.hm = self.vreg['hooks']
+        self.hm = hook.HooksManager(self.vreg)
 
     # internals ###############################################################
 
--- a/server/test/unittest_hook.py	Wed Mar 31 09:45:14 2010 +0200
+++ b/server/test/unittest_hook.py	Wed Mar 31 09:55:19 2010 +0200
@@ -81,7 +81,7 @@
         raise HookCalled()
 
 
-class HooksManagerTC(TestCase):
+class HooksRegistryTC(TestCase):
 
     def setUp(self):
         """ called before each test from this class """
@@ -115,7 +115,6 @@
                          is_hook_activated=lambda x, cls: cls.category not in dis)
         self.assertRaises(HookCalled,
                           self.o.call_hooks, 'before_add_entity', cw)
-        self.o.call_hooks('before_delete_entity', cw) # nothing to call
         dis.add('cat1')
         self.o.call_hooks('before_add_entity', cw) # disabled hooks category, not called
         dis.remove('cat1')
--- a/vregistry.py	Wed Mar 31 09:45:14 2010 +0200
+++ b/vregistry.py	Wed Mar 31 09:55:19 2010 +0200
@@ -82,6 +82,11 @@
         return cls.id
     return cls.__regid__
 
+def class_registries(cls, registryname):
+    if registryname:
+        return (registryname,)
+    return cls.__registries__
+
 
 class Registry(dict):
 
@@ -318,31 +323,32 @@
                 if obj.__module__ != modname or obj in butclasses:
                     continue
                 oid = class_regid(obj)
-                registryname = obj.__registry__
             except AttributeError:
                 continue
             if oid and not '__abstract__' in obj.__dict__:
-                self.register(obj, registryname)
+                self.register(obj, oid=oid)
 
     def register(self, obj, registryname=None, oid=None, clear=False):
         """base method to add an object in the registry"""
         assert not '__abstract__' in obj.__dict__
-        registryname = registryname or obj.__registry__
-        registry = self.setdefault(registryname)
-        registry.register(obj, oid=oid, clear=clear)
         try:
             vname = obj.__name__
         except AttributeError:
             vname = obj.__class__.__name__
-        self.debug('registered appobject %s in registry %s with id %s',
-                   vname, registryname, oid or class_regid(obj))
+        for registryname in class_registries(obj, registryname):
+            registry = self.setdefault(registryname)
+            registry.register(obj, oid=oid, clear=clear)
+            self.debug('registered appobject %s in registry %s with id %s',
+                       vname, registryname, oid or class_regid(obj))
         self._loadedmods[obj.__module__][classid(obj)] = obj
 
     def unregister(self, obj, registryname=None):
-        self[registryname or obj.__registry__].unregister(obj)
+        for registryname in class_registries(obj, registryname):
+            self[registryname].unregister(obj)
 
     def register_and_replace(self, obj, replaced, registryname=None):
-        self[registryname or obj.__registry__].register_and_replace(obj, replaced)
+        for registryname in class_registries(obj, registryname):
+            self[registryname].register_and_replace(obj, replaced)
 
     # initialization methods ###################################################
 
@@ -451,7 +457,7 @@
             self._load_ancestors_then_object(modname, parent)
         if (appobjectcls.__dict__.get('__abstract__')
             or appobjectcls.__name__[0] == '_'
-            or not appobjectcls.__registry__
+            or not appobjectcls.__registries__
             or not class_regid(appobjectcls)):
             return
         try: