[Patchew-devel] [PATCH] optimize mod

Fam Zheng famz at redhat.com
Tue Nov 20 01:33:56 UTC 2018


On Mon, 11/19 17:01, Paolo Bonzini wrote:
> On 19/11/18 14:38, Paolo Bonzini wrote:
> > Avoid looking up the module table repeatedly every time a hook is invoked,
> > by memoizing the list of hooks.  As a small additional optimization reuse
> > the result of get_or_create instead of querying the table again.
> > 
> > Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
> 
> Hmm, this undoes thw "Always read from DB to accept configuration
> updates" part, because .enabled() also comes from the DB.
> 
> One idea could be to cache enabled() every time a module is saved.
> 
> By the way, the enabled() check already doesn't work for the
> www_url_hook, which is run only ones.  More precisely, it only works
> because modules are default-enabled.

I'm in favor of dropping the enabled field from the model if it has a
performance advantage.

> 
> Paolo
> 
> > ---
> >  mod.py | 23 ++++++++++++++---------
> >  1 file changed, 14 insertions(+), 9 deletions(-)
> > 
> > diff --git a/mod.py b/mod.py
> > index e92377a..1da12ec 100644
> > --- a/mod.py
> > +++ b/mod.py
> > @@ -26,8 +26,7 @@ class PatchewModule(object):
> >      def get_model(self):
> >          # ALways read from DB to accept configuration update in-flight
> >          from api.models import Module as PC
> > -        _module_init_config(self.__class__)
> > -        return PC.objects.get(name=self.name)
> > +        return _module_init_config(self.__class__)
> >  
> >      def enabled(self):
> >          try:
> > @@ -186,14 +185,20 @@ def load_modules():
> >              _loaded_modules[cls.name] = cls()
> >              print("Loaded module:", cls.name)
> >  
> > +memoized_hooks = {}
> >  def dispatch_module_hook(hook_name, **params):
> > -    for i in [x for x in list(_loaded_modules.values()) if x.enabled()]:
> > -        if hasattr(i, hook_name):
> > -            try:
> > -                getattr(i, hook_name)(**params)
> > -            except:
> > -                print("Cannot invoke module hook: %s.%s" % (i, hook_name))
> > -                traceback.print_exc()
> > +    if not hook_name in memoized_hooks:

Nit: I think PEP wants 'hook_name not in ...'?

> > +        # The dictionary stores "bound method" objects
> > +        memoized_hooks[hook_name] = [getattr(x, hook_name)
> > +            for x in list(_loaded_modules.values())
> > +            if x.enabled()
> > +            and hasattr(x, hook_name)]
> > +    for f in memoized_hooks[hook_name]:
> > +        try:
> > +            f(**params)
> > +        except:
> > +            print("Cannot invoke module hook: %s.%s" % (type(f.__self__), hook_name))
> > +            traceback.print_exc()
> >  
> >  def get_module(name):
> >      return _loaded_modules.get(name)
> > 
> 
> _______________________________________________
> Patchew-devel mailing list
> Patchew-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/patchew-devel

Fam




More information about the Patchew-devel mailing list