[Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib

[Jan Cholasta]

Dne 2.11.2011 11:42, Alexander Bokovoy napsal(a):
> On Mon, 31 Oct 2011, Jan Cholasta wrote:
>> Dne 31.10.2011 13:19, Alexander Bokovoy napsal(a):
>>> On Mon, 31 Oct 2011, Jan Cholasta wrote:
>>>> Added finalization for __call__ and the check for CLI. Patch attached.
>>> ACK from my side but see below.
>>>
>>>> +    def __getattribute__(self, name):
>>>> +        if not name.startswith('_Plugin__') and not name.startswith('_ReadOnly__') and name != 'finalize_late':
>>>> +            self.finalize_late()
>>>> +        return object.__getattribute__(self, name)
>>> Could you get faster than three string comparisons? As
>>> __getattribute__ is fairly often called it would make sense to keep
>>> these operations to absolute minimum.
>>>
>>
>> Is there any noticable slowdown?
> Yes. Now I have different patch to solve this issue that avoids using
> __getattribute__. Instead, it sets a trap on attributes that are
> changed by finalization process and when they are accessed first time,
> the trap forces instance to finalize. As result, the attributes get
> their proper values and traps are removed, no performance costs
> anymore.
>
> For Commands one additional check is done on __call__() method to
> verify that we are indeed finalized before execution proceeds. It is a
> safety net here.
>
> Performance is not bad:
>
> 1. Before the patch
>      [root at vm-114 ipalib]# time ipa>/dev/null
>
>      real 0m1.101s
>      user 0m0.930s
>      sys 0m0.151s
>      [root at vm-114 ipalib]# time ipa user-find>/dev/null
>
>      real 0m3.132s
>      user 0m0.983s
>      sys 0m0.135s
>
> 2. With patch
>      [root at vm-114 ipalib]# patch -p2<~/speedup.patch
>      patching file frontend.py
>      patching file plugable.py
>      [root at vm-114 ipalib]# time ipa>/dev/null
>
>      real 0m0.563s
>      user 0m0.438s
>      sys 0m0.098s
>      [root at vm-114 ipalib]# time ipa>/dev/null
>
>      real 0m0.521s
>      user 0m0.412s
>      sys 0m0.100s
>      [root at vm-114 ipalib]# time ipa user-find>/dev/null
>
>      real 0m1.069s
>      user 0m0.445s
>      sys 0m0.111s
>      [root at vm-114 ipalib]# time ipa user-find>/dev/null
>
>      real 0m0.840s
>      user 0m0.425s
>      sys 0m0.126s
>      [root at vm-114 ipalib]# time ipa user-find>/dev/null
>
>      real 0m0.816s
>      user 0m0.432s
>      sys 0m0.119s
>
> Patch is attached.

I see a number of problems with the patch:

   * Only Command subclasses are finalized (that might be the reason why 
it is so fast)
   * You assume that the first operation on a plugin instance is a call 
on the args/options/params namespaces, but there is no guarantee that it 
will be
   * The whole approach is error-prone, as it requires the programmer to 
carefully setup the attributes so that the plugin can be properly 
finalized (this should be done automatically, so that every plugin is 
guaranteed to be finalized without complicated setup)

Honza

-- 
Jan Cholasta