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@vm-114 ipalib]# time ipa>/dev/null

     real 0m1.101s
     user 0m0.930s
     sys 0m0.151s
     [root@vm-114 ipalib]# time ipa user-find>/dev/null

     real 0m3.132s
     user 0m0.983s
     sys 0m0.135s

2. With patch
     [root@vm-114 ipalib]# patch -p2<~/speedup.patch
     patching file frontend.py
     patching file plugable.py
     [root@vm-114 ipalib]# time ipa>/dev/null

     real 0m0.563s
     user 0m0.438s
     sys 0m0.098s
     [root@vm-114 ipalib]# time ipa>/dev/null

     real 0m0.521s
     user 0m0.412s
     sys 0m0.100s
     [root@vm-114 ipalib]# time ipa user-find>/dev/null

     real 0m1.069s
     user 0m0.445s
     sys 0m0.111s
     [root@vm-114 ipalib]# time ipa user-find>/dev/null

     real 0m0.840s
     user 0m0.425s
     sys 0m0.126s
     [root@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

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to