Dne 9.4.2015 v 13:56 Petr Vobornik napsal(a):
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.

Thanks, ACK.

Pushed to master: e4930b3235e5d61d227a7e43d30a8feb7f35664d

--
Jan Cholasta

--
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