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@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
>
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
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
