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

[Petr Vobornik]

On 04/02/2015 11:54 AM, Petr Viktorin wrote:
> 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?

I also had to fix creation of AVAs to support utf-8 encoded str as input 
for attr and value (separately).

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

Fixed

>
> 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__`.)

Fixed

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

Fixed, but kept part for EditableDN

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

messages fixed

>
> You removed the newline at end of file.
>

line readded
-- 
Petr Vobornik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pvoborni-0811-1-performance-faster-DN-implementation.patch
Type: text/x-patch
Size: 37436 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150409/50c7fd12/attachment.bin>