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

[Petr Vobornik]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pvoborni-0809-1-speed-up-convert_attribute_members.patch
Type: text/x-patch
Size: 2892 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150409/8fcf66d4/attachment.bin>