Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
Jan Cholasta wrote:
Dne 15.11.2011 20:10, Rob Crittenden napsal(a):
Jan Cholasta wrote:
Dne 9.11.2011 11:59, Alexander Bokovoy napsal(a):
On Wed, 09 Nov 2011, Jan Cholasta wrote:
would't suffer from a facelift - currently it is messy, hard to read
code, with bits nobody ever used or uses anymore, some of the
docstrings and comments aren't correct, etc.)
A review of API class is a good idea. However, I think it is
currently
enough what is in proposed patch as it gets you 2-3x speed increase
already.
I've already started experimenting with the API class, hopefully it
will result in something useful :)
Good.
This is something I'd like to keep as it has great value for all
end-users and it requires loading all Python files in ipalib/plugins
(and in ipaserver/plugins for server side).
I'm not suggesting we should skip importing the modules. The plugin
initialization process consists of these 3 steps:
1. import plugin modules
2. create plugin instances
3. finalize plugin instances
What I'm suggesting is that we do steps 2 and 3 on-demand. We can't
do step 1 on-demand, because we have no way of knowing what plugins
are available without importing the modules. (Both your and my patch
does only step 3 on-demand, but I think we can do better than that.)
Agreed.
Anyway, if we decide to go with your solution, please make sure
*all* the plugins that are used are finalized (because of the
locking) and add a new api.env attribute which controls the lazy
finalization, so that the behavior can be overriden (actually I have
already done that - see attached patch - just use
"api.env.plugins_on_demand" instead of "api.env.context == 'cli'").
Done.
+ if not self.__dict__['_Plugin__finalized']:
+ self.finalize()
This doesn't seem right, coding style-wise. If you want to access
the property from outside the Plugin class, why did you name-mangle
it in the first place?
It is supposed to be internal detail of a Plugin. I agree it stinks a
bit here. :)
def finalize(self):
"""
"""
+ if not self.__finalized:
+ self.__finalized = True
+ else:
+ return
if not is_production_mode(self):
lock(self)
Finalize is supposed to be called just once, so IMO it would be
better not to do the check and let it throw the exception.
On contrary, I think sequential finalize() calls should do nothing.
This is at least what I expect from a finalized object -- no-op.
In my patch, finalize() throws an exception if called more than once,
but ensure_finalized() does nothing for plugins that are already
finalized. So there are both kinds of behavior available.
+ for i in ('Object', 'Property', 'Backend'):
+ if i in self:
+ namespaces.append(self[i])
AFAIK the framework is supposed to be generally usable for other
projects in the future. With that in mind, I think we can't afford
to hard-code things like this.
That's true. As you managed to get around without hardcoding, we can
drop this part.
Anyway, I've made a patch that uses data descriptors for the trap
objects (which is IMHO more elegant than what you do). I've also
managed to add on-demand finalization to Object and Attribute
subclasses by moving the set-up code from set_api() to finalize()
(it doesn't add much performance, though). See attachment.
I read through the code and I like it! Did you get the whole test
suite running with it? There are some parts of tests that expect
non-finalized objects to have None properties while they are now not
None.
I always run the test suite ;-)
All the unit tests succeed (they do when run with my patch 54 applied,
which fixes failures of some unrelated unit tests).
If so, preliminary ACK for your patch from reading it if you would
make sure on_finalize() allows multiple calls and would make a better
commit message. ;)
on_finalize() shouldn't be called directly (perhaps I should rename it
to _on_finalize to emphasize that...?)
I'll work on the commit message. As usual, it is the hardest part of the
patch for me.
I'll try to arrange testing myself today/tomorrow.
The numbers from my VM:
master abbra jcholast
$ time ipa
real 0m1.288s 0m0.619s 0m0.601s
user 0m1.103s 0m0.478s 0m0.451s
sys 0m0.161s 0m0.126s 0m0.133s
$ time ipa user-find
real 0m1.897s 0m1.253s 0m1.235s
user 0m1.119s 0m0.486s 0m0.488s
sys 0m0.160s 0m0.160s 0m0.136s
$ time ipa help
real 0m1.299s 0m0.629s 0m0.600s
user 0m1.094s 0m0.477s 0m0.446s
sys 0m0.183s 0m0.136s 0m0.140s
Looks good (your VM is supposedly at the same farm as mine so numbers
are comparable). There is anyway great variation in execution times in
VMs so 600ms vs 629ms is roughly the same.
Yep.
Thanks a lot! I think it is great advance over the original approach.
Thanks for the kind words :-)
Honza
Just a couple of questions/clarifications:
1. I think more documentation is needed for on_finalize(). The name
isn't particularly descriptive. If I read it properly you are providing
an alternate way to override finalize(), right?
Yes. I have split finalize() to finalize() (the
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
Dne 15.11.2011 20:10, Rob Crittenden napsal(a):
Jan Cholasta wrote:
Dne 9.11.2011 11:59, Alexander Bokovoy napsal(a):
On Wed, 09 Nov 2011, Jan Cholasta wrote:
would't suffer from a facelift - currently it is messy, hard to read
code, with bits nobody ever used or uses anymore, some of the
docstrings and comments aren't correct, etc.)
A review of API class is a good idea. However, I think it is currently
enough what is in proposed patch as it gets you 2-3x speed increase
already.
I've already started experimenting with the API class, hopefully it
will result in something useful :)
Good.
This is something I'd like to keep as it has great value for all
end-users and it requires loading all Python files in ipalib/plugins
(and in ipaserver/plugins for server side).
I'm not suggesting we should skip importing the modules. The plugin
initialization process consists of these 3 steps:
1. import plugin modules
2. create plugin instances
3. finalize plugin instances
What I'm suggesting is that we do steps 2 and 3 on-demand. We can't
do step 1 on-demand, because we have no way of knowing what plugins
are available without importing the modules. (Both your and my patch
does only step 3 on-demand, but I think we can do better than that.)
Agreed.
Anyway, if we decide to go with your solution, please make sure
*all* the plugins that are used are finalized (because of the
locking) and add a new api.env attribute which controls the lazy
finalization, so that the behavior can be overriden (actually I have
already done that - see attached patch - just use
"api.env.plugins_on_demand" instead of "api.env.context == 'cli'").
Done.
+ if not self.__dict__['_Plugin__finalized']:
+ self.finalize()
This doesn't seem right, coding style-wise. If you want to access
the property from outside the Plugin class, why did you name-mangle
it in the first place?
It is supposed to be internal detail of a Plugin. I agree it stinks a
bit here. :)
def finalize(self):
"""
"""
+ if not self.__finalized:
+ self.__finalized = True
+ else:
+ return
if not is_production_mode(self):
lock(self)
Finalize is supposed to be called just once, so IMO it would be
better not to do the check and let it throw the exception.
On contrary, I think sequential finalize() calls should do nothing.
This is at least what I expect from a finalized object -- no-op.
In my patch, finalize() throws an exception if called more than once,
but ensure_finalized() does nothing for plugins that are already
finalized. So there are both kinds of behavior available.
+ for i in ('Object', 'Property', 'Backend'):
+ if i in self:
+ namespaces.append(self[i])
AFAIK the framework is supposed to be generally usable for other
projects in the future. With that in mind, I think we can't afford
to hard-code things like this.
That's true. As you managed to get around without hardcoding, we can
drop this part.
Anyway, I've made a patch that uses data descriptors for the trap
objects (which is IMHO more elegant than what you do). I've also
managed to add on-demand finalization to Object and Attribute
subclasses by moving the set-up code from set_api() to finalize()
(it doesn't add much performance, though). See attachment.
I read through the code and I like it! Did you get the whole test
suite running with it? There are some parts of tests that expect
non-finalized objects to have None properties while they are now not
None.
I always run the test suite ;-)
All the unit tests succeed (they do when run with my patch 54 applied,
which fixes failures of some unrelated unit tests).
If so, preliminary ACK for your patch from reading it if you would
make sure on_finalize() allows multiple calls and would make a better
commit message. ;)
on_finalize() shouldn't be called directly (perhaps I should rename it
to _on_finalize to emphasize that...?)
I'll work on the commit message. As usual, it is the hardest part of the
patch for me.
I'll try to arrange testing myself today/tomorrow.
The numbers from my VM:
master abbra jcholast
$ time ipa
real 0m1.288s 0m0.619s 0m0.601s
user 0m1.103s 0m0.478s 0m0.451s
sys 0m0.161s 0m0.126s 0m0.133s
$ time ipa user-find
real 0m1.897s 0m1.253s 0m1.235s
user 0m1.119s 0m0.486s 0m0.488s
sys 0m0.160s 0m0.160s 0m0.136s
$ time ipa help
real 0m1.299s 0m0.629s 0m0.600s
user 0m1.094s 0m0.477s 0m0.446s
sys 0m0.183s 0m0.136s 0m0.140s
Looks good (your VM is supposedly at the same farm as mine so numbers
are comparable). There is anyway great variation in execution times in
VMs so 600ms vs 629ms is roughly the same.
Yep.
Thanks a lot! I think it is great advance over the original approach.
Thanks for the kind words :-)
Honza
Just a couple of questions/clarifications:
1. I think more documentation is needed for on_finalize(). The name
isn't particularly descriptive. If I read it properly you are providing
an alternate way to override finalize(), right?
Yes. I have split finalize() to finalize() (the method to be called
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
Jan Cholasta wrote:
Dne 9.11.2011 11:59, Alexander Bokovoy napsal(a):
On Wed, 09 Nov 2011, Jan Cholasta wrote:
would't suffer from a facelift - currently it is messy, hard to read
code, with bits nobody ever used or uses anymore, some of the
docstrings and comments aren't correct, etc.)
A review of API class is a good idea. However, I think it is currently
enough what is in proposed patch as it gets you 2-3x speed increase
already.
I've already started experimenting with the API class, hopefully it
will result in something useful :)
Good.
This is something I'd like to keep as it has great value for all
end-users and it requires loading all Python files in ipalib/plugins
(and in ipaserver/plugins for server side).
I'm not suggesting we should skip importing the modules. The plugin
initialization process consists of these 3 steps:
1. import plugin modules
2. create plugin instances
3. finalize plugin instances
What I'm suggesting is that we do steps 2 and 3 on-demand. We can't
do step 1 on-demand, because we have no way of knowing what plugins
are available without importing the modules. (Both your and my patch
does only step 3 on-demand, but I think we can do better than that.)
Agreed.
Anyway, if we decide to go with your solution, please make sure
*all* the plugins that are used are finalized (because of the
locking) and add a new api.env attribute which controls the lazy
finalization, so that the behavior can be overriden (actually I have
already done that - see attached patch - just use
"api.env.plugins_on_demand" instead of "api.env.context == 'cli'").
Done.
+ if not self.__dict__['_Plugin__finalized']:
+ self.finalize()
This doesn't seem right, coding style-wise. If you want to access
the property from outside the Plugin class, why did you name-mangle
it in the first place?
It is supposed to be internal detail of a Plugin. I agree it stinks a
bit here. :)
def finalize(self):
"""
"""
+ if not self.__finalized:
+ self.__finalized = True
+ else:
+ return
if not is_production_mode(self):
lock(self)
Finalize is supposed to be called just once, so IMO it would be
better not to do the check and let it throw the exception.
On contrary, I think sequential finalize() calls should do nothing.
This is at least what I expect from a finalized object -- no-op.
In my patch, finalize() throws an exception if called more than once,
but ensure_finalized() does nothing for plugins that are already
finalized. So there are both kinds of behavior available.
+ for i in ('Object', 'Property', 'Backend'):
+ if i in self:
+ namespaces.append(self[i])
AFAIK the framework is supposed to be generally usable for other
projects in the future. With that in mind, I think we can't afford
to hard-code things like this.
That's true. As you managed to get around without hardcoding, we can
drop this part.
Anyway, I've made a patch that uses data descriptors for the trap
objects (which is IMHO more elegant than what you do). I've also
managed to add on-demand finalization to Object and Attribute
subclasses by moving the set-up code from set_api() to finalize()
(it doesn't add much performance, though). See attachment.
I read through the code and I like it! Did you get the whole test
suite running with it? There are some parts of tests that expect
non-finalized objects to have None properties while they are now not
None.
I always run the test suite ;-)
All the unit tests succeed (they do when run with my patch 54 applied,
which fixes failures of some unrelated unit tests).
If so, preliminary ACK for your patch from reading it if you would
make sure on_finalize() allows multiple calls and would make a better
commit message. ;)
on_finalize() shouldn't be called directly (perhaps I should rename it
to _on_finalize to emphasize that...?)
I'll work on the commit message. As usual, it is the hardest part of the
patch for me.
I'll try to arrange testing myself today/tomorrow.
The numbers from my VM:
master abbra jcholast
$ time ipa
real 0m1.288s 0m0.619s 0m0.601s
user 0m1.103s 0m0.478s 0m0.451s
sys 0m0.161s 0m0.126s 0m0.133s
$ time ipa user-find
real 0m1.897s 0m1.253s 0m1.235s
user 0m1.119s 0m0.486s 0m0.488s
sys 0m0.160s 0m0.160s 0m0.136s
$ time ipa help
real 0m1.299s 0m0.629s 0m0.600s
user 0m1.094s 0m0.477s 0m0.446s
sys 0m0.183s 0m0.136s 0m0.140s
Looks good (your VM is supposedly at the same farm as mine so numbers
are comparable). There is anyway great variation in execution times in
VMs so 600ms vs 629ms is roughly the same.
Yep.
Thanks a lot! I think it is great advance over the original approach.
Thanks for the kind words :-)
Honza
Just a couple of questions/clarifications:
1. I think more documentation is needed for on_finalize(). The name
isn't particularly descriptive. If I read it properly you are providing
an alternate way to override finalize(), right?
2. Changing to a partition in Attribute is not equivalent to the regular
expression previously used. Why loosen th
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
Dne 9.11.2011 11:59, Alexander Bokovoy napsal(a):
On Wed, 09 Nov 2011, Jan Cholasta wrote:
would't suffer from a facelift - currently it is messy, hard to read
code, with bits nobody ever used or uses anymore, some of the
docstrings and comments aren't correct, etc.)
A review of API class is a good idea. However, I think it is currently
enough what is in proposed patch as it gets you 2-3x speed increase
already.
I've already started experimenting with the API class, hopefully it
will result in something useful :)
Good.
This is something I'd like to keep as it has great value for all
end-users and it requires loading all Python files in ipalib/plugins
(and in ipaserver/plugins for server side).
I'm not suggesting we should skip importing the modules. The plugin
initialization process consists of these 3 steps:
1. import plugin modules
2. create plugin instances
3. finalize plugin instances
What I'm suggesting is that we do steps 2 and 3 on-demand. We can't
do step 1 on-demand, because we have no way of knowing what plugins
are available without importing the modules. (Both your and my patch
does only step 3 on-demand, but I think we can do better than that.)
Agreed.
Anyway, if we decide to go with your solution, please make sure
*all* the plugins that are used are finalized (because of the
locking) and add a new api.env attribute which controls the lazy
finalization, so that the behavior can be overriden (actually I have
already done that - see attached patch - just use
"api.env.plugins_on_demand" instead of "api.env.context == 'cli'").
Done.
+if not self.__dict__['_Plugin__finalized']:
+self.finalize()
This doesn't seem right, coding style-wise. If you want to access
the property from outside the Plugin class, why did you name-mangle
it in the first place?
It is supposed to be internal detail of a Plugin. I agree it stinks a
bit here. :)
def finalize(self):
"""
"""
+if not self.__finalized:
+self.__finalized = True
+else:
+return
if not is_production_mode(self):
lock(self)
Finalize is supposed to be called just once, so IMO it would be
better not to do the check and let it throw the exception.
On contrary, I think sequential finalize() calls should do nothing.
This is at least what I expect from a finalized object -- no-op.
In my patch, finalize() throws an exception if called more than once,
but ensure_finalized() does nothing for plugins that are already
finalized. So there are both kinds of behavior available.
+for i in ('Object', 'Property', 'Backend'):
+if i in self:
+namespaces.append(self[i])
AFAIK the framework is supposed to be generally usable for other
projects in the future. With that in mind, I think we can't afford
to hard-code things like this.
That's true. As you managed to get around without hardcoding, we can
drop this part.
Anyway, I've made a patch that uses data descriptors for the trap
objects (which is IMHO more elegant than what you do). I've also
managed to add on-demand finalization to Object and Attribute
subclasses by moving the set-up code from set_api() to finalize()
(it doesn't add much performance, though). See attachment.
I read through the code and I like it! Did you get the whole test
suite running with it? There are some parts of tests that expect
non-finalized objects to have None properties while they are now not
None.
I always run the test suite ;-)
All the unit tests succeed (they do when run with my patch 54 applied,
which fixes failures of some unrelated unit tests).
If so, preliminary ACK for your patch from reading it if you would
make sure on_finalize() allows multiple calls and would make a better
commit message. ;)
on_finalize() shouldn't be called directly (perhaps I should rename it
to _on_finalize to emphasize that...?)
I'll work on the commit message. As usual, it is the hardest part of the
patch for me.
I'll try to arrange testing myself today/tomorrow.
The numbers from my VM:
master abbra jcholast
$ time ipa
real0m1.288s0m0.619s0m0.601s
user0m1.103s0m0.478s0m0.451s
sys 0m0.161s0m0.126s0m0.133s
$ time ipa user-find
real0m1.897s0m1.253s0m1.235s
user0m1.119s0m0.486s0m0.488s
sys 0m0.160s0m0.160s0m0.136s
$ time ipa help
real0m1.299s0m0.629s0m0.600s
user0m1.094s0m0.477s0m0.446s
sys 0m0.183s0m0.136s0m0.140s
Looks good (your VM is supposedly at the same farm as mine so numbers
are comparable). There is anyway great variation in execution times in
VMs so 600ms vs 629ms is roughly the same.
Yep.
Thanks a lot! I think it is great advance over the original approach.
Thanks for the kind words :-)
Honza
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
ht
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
On Wed, 09 Nov 2011, Jan Cholasta wrote:
> >>would't suffer from a facelift - currently it is messy, hard to read
> >>code, with bits nobody ever used or uses anymore, some of the
> >>docstrings and comments aren't correct, etc.)
> >A review of API class is a good idea. However, I think it is currently
> >enough what is in proposed patch as it gets you 2-3x speed increase
> >already.
>
> I've already started experimenting with the API class, hopefully it
> will result in something useful :)
Good.
> >This is something I'd like to keep as it has great value for all
> >end-users and it requires loading all Python files in ipalib/plugins
> >(and in ipaserver/plugins for server side).
>
> I'm not suggesting we should skip importing the modules. The plugin
> initialization process consists of these 3 steps:
>
> 1. import plugin modules
> 2. create plugin instances
> 3. finalize plugin instances
>
> What I'm suggesting is that we do steps 2 and 3 on-demand. We can't
> do step 1 on-demand, because we have no way of knowing what plugins
> are available without importing the modules. (Both your and my patch
> does only step 3 on-demand, but I think we can do better than that.)
Agreed.
> >>Anyway, if we decide to go with your solution, please make sure
> >>*all* the plugins that are used are finalized (because of the
> >>locking) and add a new api.env attribute which controls the lazy
> >>finalization, so that the behavior can be overriden (actually I have
> >>already done that - see attached patch - just use
> >>"api.env.plugins_on_demand" instead of "api.env.context == 'cli'").
> >Done.
> >
>
> +if not self.__dict__['_Plugin__finalized']:
> +self.finalize()
>
> This doesn't seem right, coding style-wise. If you want to access
> the property from outside the Plugin class, why did you name-mangle
> it in the first place?
It is supposed to be internal detail of a Plugin. I agree it stinks a
bit here. :)
> def finalize(self):
> """
> """
> +if not self.__finalized:
> +self.__finalized = True
> +else:
> +return
> if not is_production_mode(self):
> lock(self)
>
> Finalize is supposed to be called just once, so IMO it would be
> better not to do the check and let it throw the exception.
On contrary, I think sequential finalize() calls should do nothing.
This is at least what I expect from a finalized object -- no-op.
> +for i in ('Object', 'Property', 'Backend'):
> +if i in self:
> +namespaces.append(self[i])
>
> AFAIK the framework is supposed to be generally usable for other
> projects in the future. With that in mind, I think we can't afford
> to hard-code things like this.
That's true. As you managed to get around without hardcoding, we can
drop this part.
> Anyway, I've made a patch that uses data descriptors for the trap
> objects (which is IMHO more elegant than what you do). I've also
> managed to add on-demand finalization to Object and Attribute
> subclasses by moving the set-up code from set_api() to finalize()
> (it doesn't add much performance, though). See attachment.
I read through the code and I like it! Did you get the whole test
suite running with it? There are some parts of tests that expect
non-finalized objects to have None properties while they are now not
None.
If so, preliminary ACK for your patch from reading it if you would
make sure on_finalize() allows multiple calls and would make a better
commit message. ;)
I'll try to arrange testing myself today/tomorrow.
> The numbers from my VM:
>
> master abbra jcholast
> $ time ipa
> real0m1.288s0m0.619s0m0.601s
> user0m1.103s0m0.478s0m0.451s
> sys 0m0.161s0m0.126s0m0.133s
>
> $ time ipa user-find
> real0m1.897s0m1.253s0m1.235s
> user0m1.119s0m0.486s0m0.488s
> sys 0m0.160s0m0.160s0m0.136s
>
> $ time ipa help
> real0m1.299s0m0.629s0m0.600s
> user0m1.094s0m0.477s0m0.446s
> sys 0m0.183s0m0.136s0m0.140s
Looks good (your VM is supposedly at the same farm as mine so numbers
are comparable). There is anyway great variation in execution times in
VMs so 600ms vs 629ms is roughly the same.
Thanks a lot! I think it is great advance over the original approach.
--
/ Alexander Bokovoy
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
Dne 3.11.2011 12:43, Alexander Bokovoy napsal(a):
On Thu, 03 Nov 2011, Jan Cholasta wrote:
The problem with the API class is that it creates all the plugin
instances every time. Both mine and your patch don't change that,
they deal only with finalization, which is the easier part of the
lazy initialization problem. Additionally, in your patch finalize()
is called only on Command subclasses, so non-Command plugins are not
ReadOnly-locked like they should (currently this is done only when
production mode is off).
I ended up with explicitly finalizing Object, Property, and Backend
objects. This, by the way, proved that major slowdown is brought by
the Command and Method instances as I've got statistically negligible
variations of execution time, within less than 1%.
We could change API so that the plugins are initialized on-demand.
That way we could probably get rid off finalize() on plugins
altogether and move the initialization code into __init__() (because
all the required information would be available on-demand) and the
locking into API. The flaw of this approach is that it would require
a number of fundamental changes, namely changing the way API class
handles plugin initialization and moving the initialization of
"static" Plugin properties (name, module, fullname, bases, label,
...) from instance level to class level. (Nonetheless, I think API
would't suffer from a facelift - currently it is messy, hard to read
code, with bits nobody ever used or uses anymore, some of the
docstrings and comments aren't correct, etc.)
A review of API class is a good idea. However, I think it is currently
enough what is in proposed patch as it gets you 2-3x speed increase
already.
I've already started experimenting with the API class, hopefully it will
result in something useful :)
One important feature I'd like to still keep in FreeIPA is ability to
extend any object and action during plugin import phase regardless of
the python source file it comes from. So far our split between 'user
methods' and 'dnsrecord methods' is mostly utilitarian as they are
implemented in their own source code files. However, nobody prevents
you from implementing all needed modifications to all classes in the
single source file and I expect this to be fairly typical to
site-specific cases.
This is something I'd like to keep as it has great value for all
end-users and it requires loading all Python files in ipalib/plugins
(and in ipaserver/plugins for server side).
I'm not suggesting we should skip importing the modules. The plugin
initialization process consists of these 3 steps:
1. import plugin modules
2. create plugin instances
3. finalize plugin instances
What I'm suggesting is that we do steps 2 and 3 on-demand. We can't do
step 1 on-demand, because we have no way of knowing what plugins are
available without importing the modules. (Both your and my patch does
only step 3 on-demand, but I think we can do better than that.)
Anyway, if we decide to go with your solution, please make sure
*all* the plugins that are used are finalized (because of the
locking) and add a new api.env attribute which controls the lazy
finalization, so that the behavior can be overriden (actually I have
already done that - see attached patch - just use
"api.env.plugins_on_demand" instead of "api.env.context == 'cli'").
Done.
+if not self.__dict__['_Plugin__finalized']:
+self.finalize()
This doesn't seem right, coding style-wise. If you want to access the
property from outside the Plugin class, why did you name-mangle it in
the first place?
def finalize(self):
"""
"""
+if not self.__finalized:
+self.__finalized = True
+else:
+return
if not is_production_mode(self):
lock(self)
Finalize is supposed to be called just once, so IMO it would be better
not to do the check and let it throw the exception.
+for i in ('Object', 'Property', 'Backend'):
+if i in self:
+namespaces.append(self[i])
AFAIK the framework is supposed to be generally usable for other
projects in the future. With that in mind, I think we can't afford to
hard-code things like this.
Anyway, I've made a patch that uses data descriptors for the trap
objects (which is IMHO more elegant than what you do). I've also managed
to add on-demand finalization to Object and Attribute subclasses by
moving the set-up code from set_api() to finalize() (it doesn't add much
performance, though). See attachment.
The numbers from my VM:
master abbra jcholast
$ time ipa
real0m1.288s0m0.619s0m0.601s
user0m1.103s0m0.478s0m0.451s
sys 0m0.161s0m0.126s0m0.133s
$ time ipa user-find
real0m1.897s0m1.253s0m1.235s
user0m1.119s0m0.486s0m0.488s
sys 0m0.160s0m0.160s0m0.136s
$ time ipa help
real0m1.299s0m0.629s0m0.600s
user0m1.094s0
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
On Thu, 03 Nov 2011, Jan Cholasta wrote:
> The problem with the API class is that it creates all the plugin
> instances every time. Both mine and your patch don't change that,
> they deal only with finalization, which is the easier part of the
> lazy initialization problem. Additionally, in your patch finalize()
> is called only on Command subclasses, so non-Command plugins are not
> ReadOnly-locked like they should (currently this is done only when
> production mode is off).
I ended up with explicitly finalizing Object, Property, and Backend
objects. This, by the way, proved that major slowdown is brought by
the Command and Method instances as I've got statistically negligible
variations of execution time, within less than 1%.
> We could change API so that the plugins are initialized on-demand.
> That way we could probably get rid off finalize() on plugins
> altogether and move the initialization code into __init__() (because
> all the required information would be available on-demand) and the
> locking into API. The flaw of this approach is that it would require
> a number of fundamental changes, namely changing the way API class
> handles plugin initialization and moving the initialization of
> "static" Plugin properties (name, module, fullname, bases, label,
> ...) from instance level to class level. (Nonetheless, I think API
> would't suffer from a facelift - currently it is messy, hard to read
> code, with bits nobody ever used or uses anymore, some of the
> docstrings and comments aren't correct, etc.)
A review of API class is a good idea. However, I think it is currently
enough what is in proposed patch as it gets you 2-3x speed increase
already.
One important feature I'd like to still keep in FreeIPA is ability to
extend any object and action during plugin import phase regardless of
the python source file it comes from. So far our split between 'user
methods' and 'dnsrecord methods' is mostly utilitarian as they are
implemented in their own source code files. However, nobody prevents
you from implementing all needed modifications to all classes in the
single source file and I expect this to be fairly typical to
site-specific cases.
This is something I'd like to keep as it has great value for all
end-users and it requires loading all Python files in ipalib/plugins
(and in ipaserver/plugins for server side).
> Anyway, if we decide to go with your solution, please make sure
> *all* the plugins that are used are finalized (because of the
> locking) and add a new api.env attribute which controls the lazy
> finalization, so that the behavior can be overriden (actually I have
> already done that - see attached patch - just use
> "api.env.plugins_on_demand" instead of "api.env.context == 'cli'").
Done.
--
/ Alexander Bokovoy
>From 44ebebc2fede6f001a826fa4047abfeb02195cac Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy
Date: Wed, 2 Nov 2011 12:24:20 +0200
Subject: [PATCH] Perform late initialization of FreeIPA plugins
https://fedorahosted.org/freeipa/ticket/1336
When plugins are loaded, instances of the provided objects and commands
are registered in the API instance. The patch changes finalization process
to apply on first access to the Command instance that would cause either
access to properties (args, options, params) or execution of the command
itself.
The patch gives 2x boost for client-side commands like help and 3x boost
for commands that go to the server side. All performance numbers are
approximate and may vary a lot.
---
ipalib/config.py |4 ++
ipalib/constants.py|1 +
ipalib/frontend.py | 57
ipalib/plugable.py | 28 +++--
makeapi|1 +
tests/test_ipalib/test_frontend.py |6 ++--
6 files changed, 90 insertions(+), 7 deletions(-)
diff --git a/ipalib/config.py b/ipalib/config.py
index
410e5f0b252fc423e9c673e4f5b62e50eaf3602e..5e3ef8d9bc529dea59890bc8974b1d455113b14c
100644
--- a/ipalib/config.py
+++ b/ipalib/config.py
@@ -492,6 +492,10 @@ class Env(object):
if 'conf_default' not in self:
self.conf_default = self._join('confdir', 'default.conf')
+# Set plugins_on_demand:
+if 'plugins_on_demand' not in self:
+self.plugins_on_demand = (self.context == 'cli')
+
def _finalize_core(self, **defaults):
"""
Complete initialization of standard IPA environment.
diff --git a/ipalib/constants.py b/ipalib/constants.py
index
6d246288b0bb6ad3509fdb62616a03d678312319..7ec897b58786b5d7047d5fbdafb655b7d71a5400
100644
--- a/ipalib/constants.py
+++ b/ipalib/constants.py
@@ -188,6 +188,7 @@ DEFAULT_CONFIG = (
('confdir', object), # Directory containing config files
('conf', object), # File containing context specific config
('conf_default', object), # File containing context independent config
+('plugins_on_demand', object), # Wheth
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
Dne 2.11.2011 17:38, Alexander Bokovoy napsal(a): (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 real0m0.883s user0m0.504s sys 0m0.136s [root@vm-114 freeipa-2-1-speedup]# time ipa user-find>/dev/null real0m0.887s user0m0.486s sys 0m0.141s [root@vm-114 freeipa-2-1-speedup]# time ipa group-find>/dev/null real0m0.933s user0m0.446s sys 0m0.169s [root@vm-114 freeipa-2-1-speedup]# time ipa help>/dev/null real0m0.624s user0m0.479s sys 0m0.133s [root@vm-114 freeipa-2-1-speedup]# time ipa group>/dev/null real0m0.612s user0m0.475s sys 0m0.126s [root@vm-114 freeipa-2-1-speedup]# time ipa user>/dev/null real0m0.617s user0m0.472s sys 0m0.131s -- / Alexander Bokovoy The problem with the API class is that it creates all the plugin instances every time. Both mine and your patch don't change that, they deal only with finalization, which is the easier part of the lazy initialization problem. Additionally, in your patch finalize() is called only on Command subclasses, so non-Command plugins are not ReadOnly-locked like they should (currently this is done only when production mode is off). We could change API so that the plugins are initialized on-demand. That way we could probably get rid off finalize() on plugins altogether and move the initialization code into __init__() (because all the required information would be available on-demand) and the locking into API. The flaw of this approach is that it would require a number of fundamental changes, namely changing the way API class handles plugin initialization and moving the initialization of "static" Plugin properties (name, module, fullname, bases, label, ...) from instance level to class level. (Nonetheless, I think API would't suffer from a facelift - currently it is messy, hard to read code, with bits nobody ever used or uses anymore, some of the docstrings and comments aren't correct, etc.) Anyway, if we decide to go with your solution, please make sure *all* the plugins that are used are finalized (because of the locking) and add a new api.env attribute which controls the lazy fi
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
On Thu, 03 Nov 2011, Martin Kosek wrote:
> 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
> 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.
I suspected this. :)
Ok, that and I protected self.__finalized reassignment in case
Plugin#finalize() got called twice -- second time the class is locked
already so self.__finalized = True will blow exception. I made it
no-op for next passes.
New patch attached. Survived fresh re-install.
--
/ Alexander Bokovoy
>From 050e75b18a2b6856d0626edbdcabed10aa841ad3 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy
Date: Wed, 2 Nov 2011 12:24:20 +0200
Subject: [PATCH] Perform late initialization of FreeIPA plugins
https://fedorahosted.org/freeipa/ticket/1336
When plugins are loaded, instances of the provided objects and commands
are registered in the API instance. The patch changes finalization process
to apply on first access to the Command instance that would cause either
access to properties (args, options, params) or execution of the command
itself.
The patch gives 2x boost for client-side commands like help and 3x boost
for commands that go to the server side. All performance numbers are
approximate and may vary a lot.
---
ipalib/frontend.py | 50
ipalib/plugable.py | 13 ++---
tests/test_ipalib/test_frontend.py |7 +++--
3 files changed, 63 insertions(+), 7 deletions(-)
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index
61e7f493f8a8e30a1a189d06cd6a69893319deaf..45d254be23d6c8b0c359c8a4e84d0658a5bf4fe7
100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -64,6 +64,44 @@ def entry_count(entry):
return num_entries
+class _MirrorTrap(object):
+"""
+_MirrorTrap is a special class to support late initialization in FreeIPA.
+_MirrorTrap instances can be used instead of None for properties that
+change their state due to Plugin#finalize() method called.
+
+As Plugin#finalize() might be computationally expensive, objects that
could not
+act without such properties, should initialize them to _MirrorTrap()
instance
+in their __init__() method so that _MirrorTrap instance would force the
object
+to finalize() before returning the value of the property.
+
+Usage pattern is following:
+ self.args = _MirrorTrap(self, 'args')
+
+Pass the reference to the object holding the attribute and the name of the
attribute.
+On first access to the attribute, _MirrorTrip will try to call object's
finalize() method
+and expects that the attribute will be re-assigned with new (proper) value.
+
+In many places in FreeIPA, an attribute gets assigned with NameSpace()
instance during
+finalize() call.
+"""
+def __init__(self, master, attr):
+self.master = master
+self.attr = attr
+
+def __call__(self, **args):
+self.master.finalize()
+# At this point master.attr points to proper object
+return getattr(self.master, self.attr)(**args)
+
+def __iter__(self):
+self.master.finalize()
+return getattr(self.master, self.attr).__iter__()
+
+def __len__(self):
+self.master.finalize()
+return getattr(self.master, self.attr).__len__()
+
class HasParam(Plugin):
"""
@@ -404,6 +442,14 @@ class Command(HasParam):
msg_summary = None
msg_truncated = _('Results are truncated, try a more specific search')
+def __init__(self):
+self.args = _MirrorTrap(self, 'args')
+self.options = _MirrorTrap(self, 'options')
+self.output_params = _MirrorTrap(self, 'output_params')
+self.output = _MirrorTrap(self, 'output')
+
+super(Command, self).__init__()
+
def __call__(self, *args, **options):
"""
Perform validation and then execute the command.
@@ -411,6 +457,10 @@ class Command(HasParam):
If not in a server context, the call will be forwarded over
XML-RPC and the executed an the nearest IPA server.
"""
+# Plugin instance must be finalized before we get to execution
+if not self.__dict__['_Plugin__finalized']:
+self.finalize()
+
params = self.args_options_2_params(*args, **options)
self.debug(
'raw: %s(%s)', self.name, ', '.join(self._repr_iter(**params))
diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index
b0e415656e0428eb164c35a2862fcfbf50883381..d1411de32fc1a888db35e1dafdf6bf4fe8d0634b
100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -207,6 +207,7 @@ class Plugin(ReadOnly):
self.label
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
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
> >
> > real0m0.883s
> > user0m0.504s
> > sys 0m0.136s
> > [root@vm-114 freeipa-2-1-speedup]# time ipa user-find >/dev/null
> >
> > real0m0.887s
> > user0m0.486s
> > sys 0m0.141s
> > [root@vm-114 freeipa-2-1-speedup]# time ipa group-find >/dev/null
> >
> > real0m0.933s
> > user0m0.446s
> > sys 0m0.169s
> > [root@vm-114 freeipa-2-1-speedup]# time ipa help >/dev/null
> >
> > real0m0.624s
> > user0m0.479s
> > sys 0m0.133s
> > [root@vm-114 freeipa-2-1-speedup]# time ipa group >/dev/null
> >
> > real0m0.612s
> > user0m0.475s
> > sys 0m0.126s
> > [root@vm-114 freeipa-2-1-speedup]# time ipa user >/dev/null
> >
> > real0m0.617s
> > user0m0.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
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
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
(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 > > real0m0.883s > user0m0.504s > sys 0m0.136s > [root@vm-114 freeipa-2-1-speedup]# time ipa user-find >/dev/null > > real0m0.887s > user0m0.486s > sys 0m0.141s > [root@vm-114 freeipa-2-1-speedup]# time ipa group-find >/dev/null > > real0m0.933s > user0m0.446s > sys 0m0.169s > [root@vm-114 freeipa-2-1-speedup]# time ipa help >/dev/null > > real0m0.624s > user0m0.479s > sys 0m0.133s > [root@vm-114 freeipa-2-1-speedup]# time ipa group >/dev/null > > real0m0.612s > user0m0.475s > sys 0m0.126s > [root@vm-114 freeipa-2-1-speedup]# time ipa user >/dev/null > > real0m0.617s > user0m0.472s > sys 0m0.131s > > -- > / Alexander Bokovoy -- / Alexander Bokovoy >From be4fc63a16e9fdd5fa1c58aa5a5267b8e06e44ce Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy Date: Wed, 2 Nov 2011 12:24:20 +0200 Subject: [PATCH] Perform late initialization of FreeIPA plugins https://fedorahosted.org/freeipa/ticket/1336 When plugins are loaded, instances of the provided objects and commands are registered in the API instance. The patch changes finalization process to apply on first access to the Command instance that would cause either access to properties (args, options, params) or execution of the command itself. The patch gives 2x boost for client-side commands like help and 3x boost for commands that go to the server side. All performance numbers are approximate and may vary a lot. --- ipalib/frontend.py | 50 ipalib/plugable.py | 12 ++--- tests/test_ipalib/test_frontend.py |7 +++-- 3 files changed, 62 insertions(+), 7 deletions(-) diff --git a/ipalib/frontend.py b/ipalib/frontend.py index 61e7f493f8a8e30a1a189d06cd6a69893319deaf..22d77372b8cb58bb2e922b3fb158ee1025b0d242 100644 --- a/ipalib/frontend.py +++ b/ipalib/frontend.py @@ -64,6 +64,44 @@ def entry_count(entry):
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
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 real0m0.883s user0m0.504s sys 0m0.136s [root@vm-114 freeipa-2-1-speedup]# time ipa user-find >/dev/null real0m0.887s user0m0.486s sys 0m0.141s [root@vm-114 freeipa-2-1-speedup]# time ipa group-find >/dev/null real0m0.933s user0m0.446s sys 0m0.169s [root@vm-114 freeipa-2-1-speedup]# time ipa help >/dev/null real0m0.624s user0m0.479s sys 0m0.133s [root@vm-114 freeipa-2-1-speedup]# time ipa group >/dev/null real0m0.612s user0m0.475s sys 0m0.126s [root@vm-114 freeipa-2-1-speedup]# time ipa user >/dev/null real0m0.617s user0m0.472s sys 0m0.131s -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
Dne 2.11.2011 16:11, Alexander Bokovoy napsal(a): On Wed, 02 Nov 2011, Jan Cholasta wrote: 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) Perhaps you need to look at why finalize() is there at all. We have certain properties that are set only during finalize() method execution. If something cannot be performed without accessing those properties, it cannot be done before finalize() is performed. Anything else is allowed. So it boils down to being able to intercept the access to those properties that change their value in effect of finalize() call. What this all means is if a plugin writer has something in mind when changing properties in his/her re-implementation of finalize() method, it is simple to mark up those properties as such and this patch gives you a simple process and means to do so. 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. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
On Wed, 02 Nov 2011, Jan Cholasta wrote: > 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) Perhaps you need to look at why finalize() is there at all. We have certain properties that are set only during finalize() method execution. If something cannot be performed without accessing those properties, it cannot be done before finalize() is performed. Anything else is allowed. So it boils down to being able to intercept the access to those properties that change their value in effect of finalize() call. What this all means is if a plugin writer has something in mind when changing properties in his/her re-implementation of finalize() method, it is simple to mark up those properties as such and this patch gives you a simple process and means to do so. 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. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
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
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
On Wed, 02 Nov 2011, Martin Kosek wrote: > Hmm, this looks impressive! It would brings us considerably faster CLI > calls. > > And again, I would feel much safer if we do these optimizations only > for CLI and let server do all the initialization right on start. Well, that could be done in API.finalize() as we would know context at that point already. > Unit tests show few errors though. But these may not be the actual > problems in the patch, it may be just wrong test assumptions: > > == > FAIL: Doctest: ipalib.crud > -- > Traceback (most recent call last): > File "/usr/lib64/python2.7/doctest.py", line 2166, in runTest > raise self.failureException(self.format_failure(new.getvalue())) > AssertionError: Failed doctest test for ipalib.crud > File "/home/mkosek/freeipa/ipalib/crud.py", line 19, in crud these are examples that doctest extracts and tries to run. I can make Finalizer class iterable and that will solve all these: > -- > File "/home/mkosek/freeipa/ipalib/crud.py", line 76, in ipalib.crud > Failed example: > list(api.Command.user_add.args) > Exception raised: > Traceback (most recent call last): > File "/usr/lib64/python2.7/doctest.py", line 1254, in __run > compileflags, 1) in test.globs > File "", line 1, in > list(api.Command.user_add.args) > TypeError: 'Finalizer' object is not iterable > -- > File "/home/mkosek/freeipa/ipalib/crud.py", line 78, in ipalib.crud > Failed example: > list(api.Command.user_add.options) > Exception raised: > Traceback (most recent call last): > File "/usr/lib64/python2.7/doctest.py", line 1254, in __run > compileflags, 1) in test.globs > File "", line 1, in > list(api.Command.user_add.options) > TypeError: 'Finalizer' object is not iterable > -- > File "/home/mkosek/freeipa/ipalib/crud.py", line 94, in ipalib.crud > Failed example: > list(api.Command.user_show.args) > Exception raised: > Traceback (most recent call last): > File "/usr/lib64/python2.7/doctest.py", line 1254, in __run > compileflags, 1) in test.globs > File "", line 1, in > list(api.Command.user_show.args) > TypeError: 'Finalizer' object is not iterable > -- > File "/home/mkosek/freeipa/ipalib/crud.py", line 96, in ipalib.crud > Failed example: > list(api.Command.user_show.options) > Exception raised: > Traceback (most recent call last): > File "/usr/lib64/python2.7/doctest.py", line 1254, in __run > compileflags, 1) in test.globs > File "", line 1, in > list(api.Command.user_show.options) > TypeError: 'Finalizer' object is not iterable > -- > File "/home/mkosek/freeipa/ipalib/crud.py", line 114, in ipalib.crud > Failed example: > list(api.Command.user_add.output_params) > Exception raised: > Traceback (most recent call last): > File "/usr/lib64/python2.7/doctest.py", line 1254, in __run > compileflags, 1) in test.globs > File "", line 1, in > list(api.Command.user_add.output_params) > TypeError: 'NoneType' object is not iterable Output params could be run through finalizer as well. > -- > File "/home/mkosek/freeipa/ipalib/crud.py", line 116, in ipalib.crud > Failed example: > list(api.Command.user_show.output_params) > Exception raised: > Traceback (most recent call last): > File "/usr/lib64/python2.7/doctest.py", line 1254, in __run > compileflags, 1) in test.globs > File "", line 1, in > list(api.Command.user_show.output_params) > TypeError: 'NoneType' object is not iterable Same here. Errors below are about assumptions that before finalizing args, options > == > FAIL: Test the ``ipalib.frontend.Command.args`` instance attribute. > -- > Traceback (most recent call last): > File "/usr/lib/python2.7/site-packages/nose/case.py", line 187, in > runTest > self.test(*self.arg) > File "/home/mkosek/freeipa/tests/test_ipalib/test_frontend.py", line > 255, in test_args > assert self.cls().args is None > AssertionError > > == > FAIL: Test the `ipalib.frontend.Command.get_output_params` method. > -- > Traceback (most recent call last)
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
On Wed, 2011-11-02 at 12:42 +0200, Alexander Bokovoy wrote:
> 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.
Hmm, this looks impressive! It would brings us considerably faster CLI
calls.
And again, I would feel much safer if we do these optimizations only
for CLI and let server do all the initialization right on start.
Unit tests show few errors though. But these may not be the actual
problems in the patch, it may be just wrong test assumptions:
==
FAIL: Doctest: ipalib.crud
--
Traceback (most recent call last):
File "/usr/lib64/python2.7/doctest.py", line 2166, in runTest
raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for ipalib.crud
File "/home/mkosek/freeipa/ipalib/crud.py", line 19, in crud
--
File "/home/mkosek/freeipa/ipalib/crud.py", line 76, in ipalib.crud
Failed example:
list(api.Command.user_add.args)
Exception raised:
Traceback (most recent call last):
File "/usr/lib64/python2.7/doctest.py", line 1254, in __run
compileflags, 1) in test.globs
File "", line 1, in
list(api.Command.user_add.args)
TypeError: 'Finalizer' object is not iterable
--
File "/home/mkosek/freeipa/ipalib/crud.py", line 78, in ipalib.crud
Failed example:
list(api.Command.user_add.options)
Exception raised:
Traceback (most recent call last):
File "/usr/lib64/python2.7/doctest.py", line 1254, in __run
compileflags, 1) in test.globs
File "", line 1, in
list(api.Command.user_add.options)
TypeError: 'Finalizer' object is not iterable
--
File "/home/mkosek/freeipa/ipalib/crud.py", line 94, in ipalib.crud
Failed example:
list(api.Command.user_show.args)
Exception raised:
Traceback (most recent call last):
File "/usr/lib64/python2.7/doctest.py", line 1254, in __run
compileflags, 1) in test.globs
File "", line 1, in
list(api.Command.user_show.args)
TypeError: 'Finalizer' object is not iterable
--
File "/home/mkosek/freeipa/ipalib/crud.py", line 96, in ipalib.crud
Failed example:
list(api.Command.user_show.options)
Exception raised:
Traceback (most recent call last):
File "/usr/lib64/python2.7/doctest.py", line 1254, in __run
compileflags, 1) in test.globs
File "", line 1
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
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.
--
/ Alexander Bokovoy
>From d28b13f9de7d41b25c51aa7c26ca2b09f8671e6b Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy
Date: Wed, 2 Nov 2011 12:24:20 +0200
Subject: [PATCH] Perform late initialization of FreeIPA plugins
https://fedorahosted.org/freeipa/ticket/1336
When plugins are loaded, instances of the provided objects and commands
are registered in the API instance. The patch changes finalization process
to apply on first access to the Command instance that would cause either
access to properties (args, options, params) or execution of the command
itself.
The patch gives 2x boost for client-side commands like help and 3x boost
for commands that go to the server side. All performance numbers are
approximate and may vary a lot.
---
ipalib/frontend.py | 20
ipalib/plugable.py |6 ++
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index
61e7f493f8a8e30a1a189d06cd6a69893319deaf..a3fed2bbf0fb4631e238fad9892fb8129dbd6ca1
100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -404,6 +404,22 @@ class Command(HasParam):
msg_summary = None
msg_truncated = _('Results are truncated, try a more specific search')
+def __init__(self):
+class Finalizer(object):
+def __init__(self, master, attr):
+self.master = master
+self.attr = attr
+
+def __call__(self):
+self.master.finalize()
+# At this point master.attr points to proper object
+return self.master.__dict__[self.attr]()
+
+self.args = Finalizer(self, 'args')
+self.options = Finalizer(self, 'options')
+self.params = Finalizer(self, 'params')
+super(Command, self).__init__()
+
def __call__(self, *args, **options):
"""
Perform validation and then execute the command.
@@ -411,6 +427,10 @@ class Command(HasParam):
If not in a server context, the call will be forwarded over
XML-RPC and the executed an the nearest IPA server.
"""
+# Plugin instance must be finalized before we get to execution
+if not self.__dict__['_Plugin__finalized']:
+self.finalize()
+
params = self.args_options_2_params(*args, **options)
self.debug(
'raw: %s(%s)', self.name, ', '.join(self._repr_iter(**params))
diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index
b0e415656e0428eb164c35a2862fcfbf50883381..ee29929c0a6de776724659e03194973937111868
100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -207,6 +207,7 @@ class Plugin(ReadOnly):
self.label
)
)
+self.__finalized = False
def __get_api(self):
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
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?
Honza
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
On Mon, 31 Oct 2011, Simo Sorce wrote:
> On Mon, 2011-10-31 at 14:19 +0200, Alexander Bokovoy wrote:
> > 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.
>
> How common it is for name to match the above expressions ?
> If they always match then yes, we have an issue with the full strings
> being compared fully each time. If they seldom match and the name
> normally differ from the very first characters then these string
> comparisons would be really quick and not too worrying.
This is Plugin class, i.e. every instance of Command and Object.
Look at 'not name.startswith() and' clause -- it forces for each non
_Plugin__/_ReadOnly__ prefixed attribute (i.e. all attributes added to
HasParam, Command, Object, and their derivatives) to go through three
comparisons before getting to the actual attribute fetching -- it means on
every
access to Object.foo, for example.
--
/ Alexander Bokovoy
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
On Mon, 2011-10-31 at 14:19 +0200, Alexander Bokovoy wrote:
> 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.
How common it is for name to match the above expressions ?
If they always match then yes, we have an issue with the full strings
being compared fully each time. If they seldom match and the name
normally differ from the very first characters then these string
comparisons would be really quick and not too worrying.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
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.
--
/ Alexander Bokovoy
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
Dne 26.10.2011 16:50, Jan Cholasta napsal(a): Dne 26.10.2011 16:39, Martin Kosek napsal(a): On Wed, 2011-10-26 at 15:52 +0200, Jan Cholasta wrote: Dne 26.10.2011 15:41, Martin Kosek napsal(a): On Wed, 2011-10-26 at 11:39 +0200, Jan Cholasta wrote: Dne 25.10.2011 22:30, Rob Crittenden napsal(a): Ondrej Hamada wrote: On 10/25/2011 04:01 PM, Martin Kosek wrote: On Tue, 2011-10-25 at 15:29 +0200, Ondrej Hamada wrote: https://fedorahosted.org/freeipa/ticket/1336 Lazy initialization of ipalib plugins is used under all contexts, not only when context = cli. Every loaded plugin is pre-finalized - a flag is set, which means, that the plugin needs to be finalized. Then every call of plugin's __gettattr__ checks the flag and finalizes the plugin if necessary. The code was implemented by jcholast. Time reduction of commands execution is quite markable: patch [s] | normal [s] | command --- 1.468 | 2.287 | ipa user-add jsmith --firt=john --last=smith 1.658 | 2.235 | ipa user-del jsmith 1.624 | 2.204 | ipa dnsrecord-find example.com Thanks for submitting the patch. Ondra, just please provide the patch in proper format (exported via command `git format-patch -M -C --patience --full-index -1' which I sent you earlier). Martin Sorry, correct version attached Wow, this is very impressive, great job Ondra and Honza! Martin, ACK from me but I'd like a second opinion. The patch is very straightforward and clean, just want to make sure we're not missing a corner case. I ran the self-tests and didn't see any problem there. Before pushing please add the ticket # to the commit. rob I've just remembered that special methods aren't looked up through __getattribute__ (see the note at http://docs.python.org/reference/datamodel.html#more-attribute-access-for-new-style-classes). That might possibly cause problems in Command and subclasses, because it uses __call__. Honza This should be investigated. All our unit tests are OK though. I am also thinking if we shouldn't do this optimization on CLI side only as the original ticket suggests. As server side loads and finalizes all plugins just once at httpd start, performance should be OK. Plus, we would know right after the start that we are ready and there is no problem with finalizing any component. Martin As we discussed earlier - are you sure it works that way? Yes. I did a few Django-based applications before and I'm pretty sure all mod_wsgi does is caching the bytecode, which is run from the beginning for each request, like what CLI does. I did few Django application myself too. Its all about how mod_wsgi is configured. You may want to learn more about WSGIDaemonProcess and WSGIImportScript directives. In IPA, we have API already prepared and finalized for every incoming request. If it does work the way you described, the worst thing that could happen is that the plugins would be finalized the first time they are used and then stay initialized until the server terminates. Honza I know, I was just being conservative. As this optimization has no effect on server performance I considered it a little bit safer to load everything at start and be sure we are OK. But its not that hard requirement. Martin OK, thanks, just wanted to be sure. Honza Added finalization for __call__ and the check for CLI. Patch attached. Honza -- Jan Cholasta >From 083f5d555d9e652463c2ed670bc08830f075d52f Mon Sep 17 00:00:00 2001 From: Jan Cholasta Date: Mon, 31 Oct 2011 06:25:58 -0400 Subject: [PATCH] Finalize plugins on-demand in CLI. ticket 1336 --- ipalib/frontend.py |1 + ipalib/plugable.py | 27 --- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/ipalib/frontend.py b/ipalib/frontend.py index 61e7f49..285049c 100644 --- a/ipalib/frontend.py +++ b/ipalib/frontend.py @@ -411,6 +411,7 @@ class Command(HasParam): If not in a server context, the call will be forwarded over XML-RPC and the executed an the nearest IPA server. """ +self.finalize_late() params = self.args_options_2_params(*args, **options) self.debug( 'raw: %s(%s)', self.name, ', '.join(self._repr_iter(**params)) diff --git a/ipalib/plugable.py b/ipalib/plugable.py index b0e4156..9f8a40e 100644 --- a/ipalib/plugable.py +++ b/ipalib/plugable.py @@ -173,6 +173,7 @@ class Plugin(ReadOnly): """ label = None +__finalize_later = False def __init__(self): self.__api = None @@ -217,12 +218,34 @@ class Plugin(ReadOnly): return self.__api api = property(__get_api) +def try_finalize(self): +if self.__api.env.context == 'cli': +self.__finalize_later = True +else: +self.__finalize_and_check() + def finalize(self): """ """ if not is_production_mode(self): lock(self) +def __finalize_and_
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
Dne 26.10.2011 16:39, Martin Kosek napsal(a): On Wed, 2011-10-26 at 15:52 +0200, Jan Cholasta wrote: Dne 26.10.2011 15:41, Martin Kosek napsal(a): On Wed, 2011-10-26 at 11:39 +0200, Jan Cholasta wrote: Dne 25.10.2011 22:30, Rob Crittenden napsal(a): Ondrej Hamada wrote: On 10/25/2011 04:01 PM, Martin Kosek wrote: On Tue, 2011-10-25 at 15:29 +0200, Ondrej Hamada wrote: https://fedorahosted.org/freeipa/ticket/1336 Lazy initialization of ipalib plugins is used under all contexts, not only when context = cli. Every loaded plugin is pre-finalized - a flag is set, which means, that the plugin needs to be finalized. Then every call of plugin's __gettattr__ checks the flag and finalizes the plugin if necessary. The code was implemented by jcholast. Time reduction of commands execution is quite markable: patch [s] | normal [s] | command --- 1.468 | 2.287 | ipa user-add jsmith --firt=john --last=smith 1.658 | 2.235 | ipa user-del jsmith 1.624 | 2.204 | ipa dnsrecord-find example.com Thanks for submitting the patch. Ondra, just please provide the patch in proper format (exported via command `git format-patch -M -C --patience --full-index -1' which I sent you earlier). Martin Sorry, correct version attached Wow, this is very impressive, great job Ondra and Honza! Martin, ACK from me but I'd like a second opinion. The patch is very straightforward and clean, just want to make sure we're not missing a corner case. I ran the self-tests and didn't see any problem there. Before pushing please add the ticket # to the commit. rob I've just remembered that special methods aren't looked up through __getattribute__ (see the note at http://docs.python.org/reference/datamodel.html#more-attribute-access-for-new-style-classes). That might possibly cause problems in Command and subclasses, because it uses __call__. Honza This should be investigated. All our unit tests are OK though. I am also thinking if we shouldn't do this optimization on CLI side only as the original ticket suggests. As server side loads and finalizes all plugins just once at httpd start, performance should be OK. Plus, we would know right after the start that we are ready and there is no problem with finalizing any component. Martin As we discussed earlier - are you sure it works that way? Yes. I did a few Django-based applications before and I'm pretty sure all mod_wsgi does is caching the bytecode, which is run from the beginning for each request, like what CLI does. I did few Django application myself too. Its all about how mod_wsgi is configured. You may want to learn more about WSGIDaemonProcess and WSGIImportScript directives. In IPA, we have API already prepared and finalized for every incoming request. If it does work the way you described, the worst thing that could happen is that the plugins would be finalized the first time they are used and then stay initialized until the server terminates. Honza I know, I was just being conservative. As this optimization has no effect on server performance I considered it a little bit safer to load everything at start and be sure we are OK. But its not that hard requirement. Martin OK, thanks, just wanted to be sure. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
On Wed, 2011-10-26 at 15:52 +0200, Jan Cholasta wrote: > Dne 26.10.2011 15:41, Martin Kosek napsal(a): > > On Wed, 2011-10-26 at 11:39 +0200, Jan Cholasta wrote: > >> Dne 25.10.2011 22:30, Rob Crittenden napsal(a): > >>> Ondrej Hamada wrote: > On 10/25/2011 04:01 PM, Martin Kosek wrote: > > On Tue, 2011-10-25 at 15:29 +0200, Ondrej Hamada wrote: > >> https://fedorahosted.org/freeipa/ticket/1336 > >> > >> Lazy initialization of ipalib plugins is used under all contexts, not > >> only when context = cli. Every loaded plugin is pre-finalized - a flag > >> is set, which means, that the plugin needs to be finalized. Then every > >> call of plugin's __gettattr__ checks the flag and finalizes the plugin > >> if necessary. The code was implemented by jcholast. Time reduction of > >> commands execution is quite markable: > >> > >> patch [s] | normal [s] | command > >> --- > >> 1.468 | 2.287 | ipa user-add jsmith --firt=john > >> --last=smith > >> 1.658 | 2.235 | ipa user-del jsmith > >> 1.624 | 2.204 | ipa dnsrecord-find example.com > >> > > Thanks for submitting the patch. Ondra, just please provide the patch in > > proper format (exported via command `git format-patch -M -C --patience > > --full-index -1' which I sent you earlier). > > > > Martin > > > > > Sorry, correct version attached > >>> > >>> Wow, this is very impressive, great job Ondra and Honza! > >>> > >>> Martin, ACK from me but I'd like a second opinion. The patch is very > >>> straightforward and clean, just want to make sure we're not missing a > >>> corner case. > >>> > >>> I ran the self-tests and didn't see any problem there. > >>> > >>> Before pushing please add the ticket # to the commit. > >>> > >>> rob > >>> > >> > >> I've just remembered that special methods aren't looked up through > >> __getattribute__ (see the note at > >> http://docs.python.org/reference/datamodel.html#more-attribute-access-for-new-style-classes). > >> That might possibly cause problems in Command and subclasses, because it > >> uses __call__. > >> > >> Honza > >> > > > > This should be investigated. All our unit tests are OK though. > > > > I am also thinking if we shouldn't do this optimization on CLI side only > > as the original ticket suggests. As server side loads and finalizes all > > plugins just once at httpd start, performance should be OK. Plus, we > > would know right after the start that we are ready and there is no > > problem with finalizing any component. > > > > Martin > > > > As we discussed earlier - are you sure it works that way? Yes. > I did a few > Django-based applications before and I'm pretty sure all mod_wsgi does > is caching the bytecode, which is run from the beginning for each > request, like what CLI does. I did few Django application myself too. Its all about how mod_wsgi is configured. You may want to learn more about WSGIDaemonProcess and WSGIImportScript directives. In IPA, we have API already prepared and finalized for every incoming request. > > If it does work the way you described, the worst thing that could happen > is that the plugins would be finalized the first time they are used and > then stay initialized until the server terminates. > > Honza > I know, I was just being conservative. As this optimization has no effect on server performance I considered it a little bit safer to load everything at start and be sure we are OK. But its not that hard requirement. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
Dne 26.10.2011 15:41, Martin Kosek napsal(a): On Wed, 2011-10-26 at 11:39 +0200, Jan Cholasta wrote: Dne 25.10.2011 22:30, Rob Crittenden napsal(a): Ondrej Hamada wrote: On 10/25/2011 04:01 PM, Martin Kosek wrote: On Tue, 2011-10-25 at 15:29 +0200, Ondrej Hamada wrote: https://fedorahosted.org/freeipa/ticket/1336 Lazy initialization of ipalib plugins is used under all contexts, not only when context = cli. Every loaded plugin is pre-finalized - a flag is set, which means, that the plugin needs to be finalized. Then every call of plugin's __gettattr__ checks the flag and finalizes the plugin if necessary. The code was implemented by jcholast. Time reduction of commands execution is quite markable: patch [s] | normal [s] | command --- 1.468 | 2.287 | ipa user-add jsmith --firt=john --last=smith 1.658 | 2.235 | ipa user-del jsmith 1.624 | 2.204 | ipa dnsrecord-find example.com Thanks for submitting the patch. Ondra, just please provide the patch in proper format (exported via command `git format-patch -M -C --patience --full-index -1' which I sent you earlier). Martin Sorry, correct version attached Wow, this is very impressive, great job Ondra and Honza! Martin, ACK from me but I'd like a second opinion. The patch is very straightforward and clean, just want to make sure we're not missing a corner case. I ran the self-tests and didn't see any problem there. Before pushing please add the ticket # to the commit. rob I've just remembered that special methods aren't looked up through __getattribute__ (see the note at http://docs.python.org/reference/datamodel.html#more-attribute-access-for-new-style-classes). That might possibly cause problems in Command and subclasses, because it uses __call__. Honza This should be investigated. All our unit tests are OK though. I am also thinking if we shouldn't do this optimization on CLI side only as the original ticket suggests. As server side loads and finalizes all plugins just once at httpd start, performance should be OK. Plus, we would know right after the start that we are ready and there is no problem with finalizing any component. Martin As we discussed earlier - are you sure it works that way? I did a few Django-based applications before and I'm pretty sure all mod_wsgi does is caching the bytecode, which is run from the beginning for each request, like what CLI does. If it does work the way you described, the worst thing that could happen is that the plugins would be finalized the first time they are used and then stay initialized until the server terminates. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
On Wed, 2011-10-26 at 11:39 +0200, Jan Cholasta wrote: > Dne 25.10.2011 22:30, Rob Crittenden napsal(a): > > Ondrej Hamada wrote: > >> On 10/25/2011 04:01 PM, Martin Kosek wrote: > >>> On Tue, 2011-10-25 at 15:29 +0200, Ondrej Hamada wrote: > https://fedorahosted.org/freeipa/ticket/1336 > > Lazy initialization of ipalib plugins is used under all contexts, not > only when context = cli. Every loaded plugin is pre-finalized - a flag > is set, which means, that the plugin needs to be finalized. Then every > call of plugin's __gettattr__ checks the flag and finalizes the plugin > if necessary. The code was implemented by jcholast. Time reduction of > commands execution is quite markable: > > patch [s] | normal [s] | command > --- > 1.468 | 2.287 | ipa user-add jsmith --firt=john > --last=smith > 1.658 | 2.235 | ipa user-del jsmith > 1.624 | 2.204 | ipa dnsrecord-find example.com > > >>> Thanks for submitting the patch. Ondra, just please provide the patch in > >>> proper format (exported via command `git format-patch -M -C --patience > >>> --full-index -1' which I sent you earlier). > >>> > >>> Martin > >>> > >>> > >> Sorry, correct version attached > > > > Wow, this is very impressive, great job Ondra and Honza! > > > > Martin, ACK from me but I'd like a second opinion. The patch is very > > straightforward and clean, just want to make sure we're not missing a > > corner case. > > > > I ran the self-tests and didn't see any problem there. > > > > Before pushing please add the ticket # to the commit. > > > > rob > > > > I've just remembered that special methods aren't looked up through > __getattribute__ (see the note at > http://docs.python.org/reference/datamodel.html#more-attribute-access-for-new-style-classes). > > That might possibly cause problems in Command and subclasses, because it > uses __call__. > > Honza > This should be investigated. All our unit tests are OK though. I am also thinking if we shouldn't do this optimization on CLI side only as the original ticket suggests. As server side loads and finalizes all plugins just once at httpd start, performance should be OK. Plus, we would know right after the start that we are ready and there is no problem with finalizing any component. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
Dne 25.10.2011 22:30, Rob Crittenden napsal(a): Ondrej Hamada wrote: On 10/25/2011 04:01 PM, Martin Kosek wrote: On Tue, 2011-10-25 at 15:29 +0200, Ondrej Hamada wrote: https://fedorahosted.org/freeipa/ticket/1336 Lazy initialization of ipalib plugins is used under all contexts, not only when context = cli. Every loaded plugin is pre-finalized - a flag is set, which means, that the plugin needs to be finalized. Then every call of plugin's __gettattr__ checks the flag and finalizes the plugin if necessary. The code was implemented by jcholast. Time reduction of commands execution is quite markable: patch [s] | normal [s] | command --- 1.468 | 2.287 | ipa user-add jsmith --firt=john --last=smith 1.658 | 2.235 | ipa user-del jsmith 1.624 | 2.204 | ipa dnsrecord-find example.com Thanks for submitting the patch. Ondra, just please provide the patch in proper format (exported via command `git format-patch -M -C --patience --full-index -1' which I sent you earlier). Martin Sorry, correct version attached Wow, this is very impressive, great job Ondra and Honza! Martin, ACK from me but I'd like a second opinion. The patch is very straightforward and clean, just want to make sure we're not missing a corner case. I ran the self-tests and didn't see any problem there. Before pushing please add the ticket # to the commit. rob I've just remembered that special methods aren't looked up through __getattribute__ (see the note at http://docs.python.org/reference/datamodel.html#more-attribute-access-for-new-style-classes). That might possibly cause problems in Command and subclasses, because it uses __call__. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
Ondrej Hamada wrote: On 10/25/2011 04:01 PM, Martin Kosek wrote: On Tue, 2011-10-25 at 15:29 +0200, Ondrej Hamada wrote: https://fedorahosted.org/freeipa/ticket/1336 Lazy initialization of ipalib plugins is used under all contexts, not only when context = cli. Every loaded plugin is pre-finalized - a flag is set, which means, that the plugin needs to be finalized. Then every call of plugin's __gettattr__ checks the flag and finalizes the plugin if necessary. The code was implemented by jcholast. Time reduction of commands execution is quite markable: patch [s] | normal [s] | command --- 1.468 | 2.287 | ipa user-add jsmith --firt=john --last=smith 1.658 | 2.235 | ipa user-del jsmith 1.624 | 2.204 | ipa dnsrecord-find example.com Thanks for submitting the patch. Ondra, just please provide the patch in proper format (exported via command `git format-patch -M -C --patience --full-index -1' which I sent you earlier). Martin Sorry, correct version attached Wow, this is very impressive, great job Ondra and Honza! Martin, ACK from me but I'd like a second opinion. The patch is very straightforward and clean, just want to make sure we're not missing a corner case. I ran the self-tests and didn't see any problem there. Before pushing please add the ticket # to the commit. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
On 10/25/2011 04:01 PM, Martin Kosek wrote:
On Tue, 2011-10-25 at 15:29 +0200, Ondrej Hamada wrote:
https://fedorahosted.org/freeipa/ticket/1336
Lazy initialization of ipalib plugins is used under all contexts, not
only when context = cli. Every loaded plugin is pre-finalized - a flag
is set, which means, that the plugin needs to be finalized. Then every
call of plugin's __gettattr__ checks the flag and finalizes the plugin
if necessary. The code was implemented by jcholast. Time reduction of
commands execution is quite markable:
patch [s] | normal [s]| command
---
1.468 | 2.287 | ipa user-add jsmith --firt=john
--last=smith
1.658 | 2.235 | ipa user-del jsmith
1.624 | 2.204 | ipa dnsrecord-find example.com
Thanks for submitting the patch. Ondra, just please provide the patch in
proper format (exported via command `git format-patch -M -C --patience
--full-index -1' which I sent you earlier).
Martin
Sorry, correct version attached
--
Regards,
Ondrej Hamada
FreeIPA team
jabber: oh...@jabbim.cz
IRC: ohamada
From 798d8f8a624f8350974e54c328c1c58c06944b26 Mon Sep 17 00:00:00 2001
From: Ondrej Hamada
Date: Tue, 25 Oct 2011 16:20:44 +0200
Subject: [PATCH] lazy initialization patch
---
ipalib/plugable.py | 18 +++---
1 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index b0e415656e0428eb164c35a2862fcfbf50883381..2aed1cdcda18840728558ed53435ab10ae28e802 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -173,6 +173,7 @@ class Plugin(ReadOnly):
"""
label = None
+__try_finalize = False
def __init__(self):
self.__api = None
@@ -208,6 +209,16 @@ class Plugin(ReadOnly):
)
)
+def __getattribute__(self, name):
+if name.startswith('_Plugin__') or name.startswith('_ReadOnly__'):
+return object.__getattribute__(self, name)
+if self.__try_finalize:
+self.__try_finalize = False
+self.finalize()
+if not is_production_mode(self.__api):
+assert islocked(self) is True
+return object.__getattribute__(self, name)
+
def __get_api(self):
"""
Return `API` instance passed to `finalize()`.
@@ -217,6 +228,9 @@ class Plugin(ReadOnly):
return self.__api
api = property(__get_api)
+def prefinalize(self):
+self.__try_finalize = True
+
def finalize(self):
"""
"""
@@ -638,9 +652,7 @@ class API(DictProxy):
assert p.instance.api is self
for p in plugins.itervalues():
-p.instance.finalize()
-if not production_mode:
-assert islocked(p.instance) is True
+p.instance.prefinalize()
object.__setattr__(self, '_API__finalized', True)
tuple(PluginInfo(p) for p in plugins.itervalues())
object.__setattr__(self, 'plugins',
--
1.7.6.4
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
On Tue, 2011-10-25 at 15:29 +0200, Ondrej Hamada wrote: > https://fedorahosted.org/freeipa/ticket/1336 > > Lazy initialization of ipalib plugins is used under all contexts, not > only when context = cli. Every loaded plugin is pre-finalized - a flag > is set, which means, that the plugin needs to be finalized. Then every > call of plugin's __gettattr__ checks the flag and finalizes the plugin > if necessary. The code was implemented by jcholast. Time reduction of > commands execution is quite markable: > > patch [s] | normal [s]| command > --- > 1.468 | 2.287 | ipa user-add jsmith --firt=john > --last=smith > 1.658 | 2.235 | ipa user-del jsmith > 1.624 | 2.204 | ipa dnsrecord-find example.com > Thanks for submitting the patch. Ondra, just please provide the patch in proper format (exported via command `git format-patch -M -C --patience --full-index -1' which I sent you earlier). Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib
https://fedorahosted.org/freeipa/ticket/1336
Lazy initialization of ipalib plugins is used under all contexts, not
only when context = cli. Every loaded plugin is pre-finalized - a flag
is set, which means, that the plugin needs to be finalized. Then every
call of plugin's __gettattr__ checks the flag and finalizes the plugin
if necessary. The code was implemented by jcholast. Time reduction of
commands execution is quite markable:
patch [s] | normal [s]| command
---
1.468 | 2.287 | ipa user-add jsmith --firt=john
--last=smith
1.658 | 2.235 | ipa user-del jsmith
1.624 | 2.204 | ipa dnsrecord-find example.com
--
Regards,
Ondrej Hamada
FreeIPA team
jabber: oh...@jabbim.cz
IRC: ohamada
diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index b0e4156..2aed1cd 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -173,6 +173,7 @@ class Plugin(ReadOnly):
"""
label = None
+__try_finalize = False
def __init__(self):
self.__api = None
@@ -208,6 +209,16 @@ class Plugin(ReadOnly):
)
)
+def __getattribute__(self, name):
+if name.startswith('_Plugin__') or name.startswith('_ReadOnly__'):
+return object.__getattribute__(self, name)
+if self.__try_finalize:
+self.__try_finalize = False
+self.finalize()
+if not is_production_mode(self.__api):
+assert islocked(self) is True
+return object.__getattribute__(self, name)
+
def __get_api(self):
"""
Return `API` instance passed to `finalize()`.
@@ -217,6 +228,9 @@ class Plugin(ReadOnly):
return self.__api
api = property(__get_api)
+def prefinalize(self):
+self.__try_finalize = True
+
def finalize(self):
"""
"""
@@ -638,9 +652,7 @@ class API(DictProxy):
assert p.instance.api is self
for p in plugins.itervalues():
-p.instance.finalize()
-if not production_mode:
-assert islocked(p.instance) is True
+p.instance.prefinalize()
object.__setattr__(self, '_API__finalized', True)
tuple(PluginInfo(p) for p in plugins.itervalues())
object.__setattr__(self, 'plugins',
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
