[Freeipa-devel] [PATCH] 811 performance: faster DN implementation

[Petr Viktorin]

On 03/31/2015 12:11 PM, Petr Vobornik wrote:
> The only different thing is a lack of utf-8 encoded str support(as
> input). I don't know how much important the support is.

I don't think that support is too important (assuming IPA doesn't use 
it!). However, the behavior with this patch is dangerous.
It allows unicode and ASCII strings, but fails on non-ASCII strings. 
That means things will usually work, but as soon as a non-ASCII 
component is introduced at the wrong place, you get an error.

Restoring support for utf-8 encoded str looks easy to do; here's a patch 
you can squash in. Or did I miss something?

> maybe it could be attached to ticket
> https://fedorahosted.org/freeipa/ticket/4947
> -----
> DN code was optimized to be faster if DNs are created from string. This
> is the major use case, since most DNs come from LDAP.
>
> With this patch, DN creation is almost 8-10x faster (with 30K-100K DNs).
>
> Second mojor use case - deepcopy in LDAPEntry is about 20x faster - done
> by custom __deepcopy__ function.
>
> The major change is that DN is no longer internally composed  of RDNs
> and AVAs but it rather keeps the data in open ldap format - the same as
> output of str2dn function. Therefore, for immutable DNs, no other
> transformations are required on instantiation.
>
> The format is:
>
> DN: [RDN, RDN,...]
> RDN: [AVA, AVA,...]
> AVA: ['utf-8 encoded str - attr', 'utf-8 encode str -value', FLAG]
> FLAG: int
>
> Further indexing of DN object constructs an RDN which is just an
> encapsulation of the RDN part of open ldap representation. Indexing of
> RDN constructs AVA in the same fashion.
>
> Obtained EditableAVA, EditableRDN from EditableDN shares the respected
> lists of the open ldap repr. so that the change of value or attr is
> reflected in parent object.


Looks good. A couple of comments:

RDN.to_openldap: _avas always has 3 components, right? I'd prefer 
`list(a)` over `[a[0], a[1], a[2]]`.  Similarly for tuple in in __add__ 
and RDN._avas_from_sequence.

DN._rdns_from_value: the error message at the end is wrong, RDN is also 
accepted. (And, `type(value)` would be more informative than 
`value.__class__.__name__`.)

You can optimize __deepcopy__ for immutable DNs even further: just 
return self!

In DN.find & rfind, RDNs are not accepted but the error message says 
they are.

You removed the newline at end of file.

-- 
Petr Viktorin

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Restore-support-for-creating-DNs-from-utf-8-encoded-.patch
Type: text/x-patch
Size: 6630 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150402/a692277d/attachment.bin>