[Freeipa-devel] [PATCH] 809 speed up convert_attribute_members

[Jan Cholasta]

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


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.


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


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


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)

Honza

-- 
Jan Cholasta