On 04/02/2015 09:47 AM, Jan Cholasta wrote:
Hi,

Dne 31.3.2015 v 12:11 Petr Vobornik napsal(a):
A workaround to avoid usage of slow LDAPEntry._sync_attr #4946.

I originally wanted to avoid DN processing as well but we can't do that
because of DNs which are encoded - e.g. contains '+' or ','. Therefore
patch 811 - faster DN implementation is very useful. Also patch 809 is
useful to avoid high load of 389.

https://fedorahosted.org/freeipa/ticket/4965


1)

+            dn = container_dns.get(ldap_obj_name, None)
+            if not dn:
+                ldap_obj = self.api.Object[ldap_obj_name]
+                dn = DN(ldap_obj.container_dn, api.env.basedn)
+                container_dns[ldap_obj_name] = dn
+            return dn

     a) The second argument of .get() is None by default

     b) "not dn" matches None as well as empty DNs, use "dn is not None"
(it's not that there could be empty DNs here, but let's not give a
potential reader the wrong idea)

     c) It would be better to catch KeyError rather than call .get() and
check the result:

             try:
                 dn = container_dns[ldap_obj_name]
             except KeyError:
                 dn = ...
                 container_dns[ldap_obj_name] = dn

Changed



2) Does get_new_attr() actually provide any speed up? Unless I'm missing
something, it just mirrors the virtual member attributes already readily
available from entry_attrs in new_attrs.

Yes, a bit. With 30K members and my vm get_new_attr takes ~ 0.114s. setdefault takes ~ 0.686s which is about 7-10% of the entire convert_attribute_members. Pure dict is faster.



3) get_container_dn() and get_new_attr() do not need to be functions,
since each is called just from a single spot.

Changed



4) "memberdn = DN(member)" could be one for loop up.


Changed


Here's what I ended up with trying to fix the above (untested):

         for attr in self.attribute_members:
             try:
                 value = entry_attrs.raw[attr]
             except KeyError:
                 continue
             del entry_attrs[attr]

             ldap_objs = {}
             for ldap_obj_name in self.attribute_members[attr]:
                 ldap_obj = self.api.Object[ldap_obj_name]
                 container_dn = DN(ldap_obj.container_dn, api.env.basedn)
                 ldap_objs[container_dn] = ldap_obj

             for member in value:
                 memberdn = DN(member)
                 try:
                     ldap_obj = ldap_objs[DN(*memberdn[1:])]
                 except KeyError:
                     continue

                 new_attr = '%s_%s' % (attr, ldap_obj.name)
                 new_value = ldap_obj.get_primary_key_from_dn(memberdn)
                 entry_attrs.setdefault(new_attr, []).append(new_value)

Without any modifications the code is ~ 2.3x slower than mine. In patch 811 DN's slice, __hash__ and __eq__ functions are optimized.


Honza

--
Petr Vobornik
From e615fde310ce7b8fc61f2d338b7e8f0f635fa175 Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Wed, 18 Mar 2015 18:48:54 +0100
Subject: [PATCH] speed up convert_attribute_members

A workaround to avoid usage of slow LDAPEntry._sync_attr #4946

https://fedorahosted.org/freeipa/ticket/4965
---
 ipalib/plugins/baseldap.py | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 4b1c701924d57919538e0c428ea181c2e898505e..c9f98ed12d3a584597af9b0be08361bceca620e7 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -631,18 +631,38 @@ class LDAPObject(Object):
     def convert_attribute_members(self, entry_attrs, *keys, **options):
         if options.get('raw', False):
             return
+
+        container_dns = {}
+        new_attrs = {}
+
         for attr in self.attribute_members:
-            for member in entry_attrs.setdefault(attr, []):
-                for ldap_obj_name in self.attribute_members[attr]:
-                    ldap_obj = self.api.Object[ldap_obj_name]
-                    container_dn = DN(ldap_obj.container_dn, api.env.basedn)
-                    if member.endswith(container_dn):
-                        new_attr = '%s_%s' % (attr, ldap_obj.name)
-                        entry_attrs.setdefault(new_attr, []).append(
-                            ldap_obj.get_primary_key_from_dn(member)
-                        )
+            try:
+                value = entry_attrs.raw[attr]
+            except KeyError:
+                continue
             del entry_attrs[attr]
 
+            for member in value:
+                memberdn = DN(member)
+                for ldap_obj_name in self.attribute_members[attr]:
+                    ldap_obj = self.api.Object[ldap_obj_name]
+                    try:
+                        container_dn = container_dns[ldap_obj_name]
+                    except KeyError:
+                        container_dn = DN(ldap_obj.container_dn, api.env.basedn)
+                        container_dns[ldap_obj_name] = container_dn
+
+                    if memberdn.endswith(container_dn):
+                        new_value = ldap_obj.get_primary_key_from_dn(memberdn)
+                        new_attr_name = '%s_%s' % (attr, ldap_obj.name)
+                        try:
+                            new_attr = new_attrs[new_attr_name]
+                        except KeyError:
+                            new_attr = entry_attrs.setdefault(new_attr_name, [])
+                            new_attrs[new_attr_name] = new_attr
+                        new_attr.append(new_value)
+                        break
+
     def get_password_attributes(self, ldap, dn, entry_attrs):
         """
         Search on the entry to determine if it has a password or
-- 
2.1.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to