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

[Martin Kosek]

On Wed, 2011-11-02 at 18:38 +0200, Alexander Bokovoy wrote:
> (actual patch attached!)
> 
> On Wed, 02 Nov 2011, Alexander Bokovoy wrote:
> > 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 at vm-114 freeipa-2-1-speedup]# time ipa dnsrecord-find ipa.local >/dev/null
> > 
> > real    0m0.883s
> > user    0m0.504s
> > sys     0m0.136s
> > [root at vm-114 freeipa-2-1-speedup]# time ipa user-find >/dev/null
> > 
> > real    0m0.887s
> > user    0m0.486s
> > sys     0m0.141s
> > [root at vm-114 freeipa-2-1-speedup]# time ipa group-find >/dev/null
> > 
> > real    0m0.933s
> > user    0m0.446s
> > sys     0m0.169s
> > [root at vm-114 freeipa-2-1-speedup]# time ipa help >/dev/null
> > 
> > real    0m0.624s
> > user    0m0.479s
> > sys     0m0.133s
> > [root at vm-114 freeipa-2-1-speedup]# time ipa group >/dev/null
> > 
> > real    0m0.612s
> > user    0m0.475s
> > sys     0m0.126s
> > [root at vm-114 freeipa-2-1-speedup]# time ipa user >/dev/null
> > 
> > real    0m0.617s
> > user    0m0.472s
> > sys     0m0.131s
> > 
> > -- 
> > / Alexander Bokovoy
> 

Good, this indeed addresses all my previous concerns but one :-)

$ ./makeapi 
Writing API to API.txt
Traceback (most recent call last):
  File "./makeapi", line 392, in <module>
    sys.exit(main())
  File "./makeapi", line 376, in main
    rval |= make_api()
  File "./makeapi", line 167, in make_api
    fd.write('args: %d,%d,%d\n' % (len(cmd.args), len(cmd.options), len(cmd.output)))

But if you change "len" functions to "__len__" it works fine.

Martin