[Patchew-devel] [PATCH] optimize mod

[Paolo Bonzini]

On 20/11/18 02:33, Fam Zheng wrote:
> 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.

Ok.

>>>  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 ...'?

Yep.

Paolo