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 <aboko...@redhat.com>
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):
         """
@@ -220,6 +221,7 @@ class Plugin(ReadOnly):
     def finalize(self):
         """
         """
+        self.__finalized = True
         if not is_production_mode(self):
             lock(self)
 
@@ -637,10 +639,6 @@ class API(DictProxy):
             if not production_mode:
                 assert p.instance.api is self
 
-        for p in plugins.itervalues():
-            p.instance.finalize()
-            if not production_mode:
-                assert islocked(p.instance) is True
         object.__setattr__(self, '_API__finalized', True)
         tuple(PluginInfo(p) for p in plugins.itervalues())
         object.__setattr__(self, 'plugins',
-- 
1.7.7

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

Reply via email to