On Wed, 02 Nov 2011, Jan Cholasta wrote: > >Callable instances are a consequence of the above -- > >Command.__call__() does use properties that are changed due to > >finalize() being run. In fact, there are so many places in FreeIPA > >where we simply expect that foo.args/params/output_params/output is > >both iterable and callable instance that it is not even funny. > > > >Now, the process established by the proposed patch is actually > >dependent only on the fact that bound instance has finalize() method. > >This is quite generic and can be used for all other types of objects. > > > > The real problem is in the API class so it should be fixed there, > not worked around in Plugin subclasses. Please explain why you do think real problem is in API class. Finalize() is needed purely to being able to split plugins' property initialization into stages before loading plugins and after due to the fact that each and every plugin could contribute commands and additions to the same objects. While these stages are separate, plugins can access properties and commands they want and it is duty of a plugin to get its property working right. We have finalize() method exactly for this purpose.
API class' attempt to call finalize() on each plugin's instance at once is a poor replacement to detecting access to not-yet-initialized properties. We can implement that access like you have proposed with __getattribute__ but it has additional cost for all other properties that don't need post-initialization (finalization) and, what's more important, they incur these costs irrespective whether initialization has happened or not. That's bad and my patch shows it is actually noticeable as the performance difference, for example, of 'ipa dnsrecord-find example.com' between the two patches is about 2x in favor of my patch (1.624s vs 0.912s). Regarding other objects, here is the list of places where we define specific finalizers: $ git grep 'def finalize' ipalib/cli.py: def finalize(self): ipalib/frontend.py: def finalize(self): ipalib/plugable.py: def finalize(self): ipalib/plugable.py: def finalize(self): ipaserver/rpcserver.py: def finalize(self): ipaserver/rpcserver.py: def finalize(self): ipaserver/rpcserver.py: def finalize(self): tests/test_ipalib/test_cli.py: def finalize(self): tests/test_ipalib/test_config.py: def finalize_core(self, ctx, **defaults): tests/util.py: def finalize(self, *plugins, **kw): As you can see, there NO plugins that define their own finalizers, thus, all of them rely on API.finalize(), help.finalize(), and Plugin.finalize()/Command.finalize(). ipaserver/rpcserver.py finalizers are fine as well, I have checked them. Updated patch is attached. It addresses all comments from Martin. [root@vm-114 freeipa-2-1-speedup]# time ipa dnsrecord-find ipa.local >/dev/null real 0m0.883s user 0m0.504s sys 0m0.136s [root@vm-114 freeipa-2-1-speedup]# time ipa user-find >/dev/null real 0m0.887s user 0m0.486s sys 0m0.141s [root@vm-114 freeipa-2-1-speedup]# time ipa group-find >/dev/null real 0m0.933s user 0m0.446s sys 0m0.169s [root@vm-114 freeipa-2-1-speedup]# time ipa help >/dev/null real 0m0.624s user 0m0.479s sys 0m0.133s [root@vm-114 freeipa-2-1-speedup]# time ipa group >/dev/null real 0m0.612s user 0m0.475s sys 0m0.126s [root@vm-114 freeipa-2-1-speedup]# time ipa user >/dev/null real 0m0.617s user 0m0.472s sys 0m0.131s -- / Alexander Bokovoy _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
