Re: ACL Performance (caching on object basis) (ITS#1523)

["Kurt D. Zeilenga" <Kurt@OpenLDAP.org>]

The approach I suggested was designed specifically to address
the common case which you raised at the top of this thread.  That
is, speeding up value independent ACL evaluation for attributes
with large number of values.  The approach, I believe, adequately
addresses this and a number of other common cases with very
little overhead.  In particular, I do not see any case where its
performance would be worse than the existing code.

With your approach, it appears that cache management overhead,
in particular the extra malloc/free calls, may offset whatever
benefit the cache might offer.  I believe further performance
analysis is needed to determine whether your approach will be
generally useful or not. I encourage you and others to undertake
such analysis as this may demonstrate your suggested approach
is viable.

In the meantime, I propose that my suggested patch be committed.

Kurt


At 12:17 AM 2002-02-04, Stephan Siano wrote:
>On Friday, 1. February 2002 22:41, Kurt D. Zeilenga wrote:
>> At 11:10 AM 2002-02-01, Howard Chu wrote:
>> >I've looked thru it, there are still some details that could be
>> >cleaned up. I think this code does too much work if all you want
>> >is to speed up send_search_entry, and there is a simpler way to
>> >accomplish that, as Kurt has described in previous posts.
>>
>> I worked up a patch to demonstrate the approach I'm advocating.
>>         ftp://ftp.openldap.org/incoming/aclstate.patch
>>
>> Consider this a prototype, I haven't had time to think
>> through all the cases nor run much more than test006
>> against it.
>>
>> The advantages of this approach are:
>>         very little change to access_allowed() callers
>>           (even those who provide a state structure)
>
>Well, the change to the access_allowed callers is not much bigger in case of 
>my patch (my structure needs to be destroyed, yours doesn't)
>
>>         no additional malloc/free calls
>
>Yes, that's clearly the advantage of your approach.
>
>>         no additional loops
>
>The only additional loops that my patch introduces are through the already 
>evaluated ACLs. Since your patch does only store one of those, you don't need 
>to loop. In my case, if only one ACL applies to the object, the loops become 
>rather short. If more than one ACL applies to the object my code is likely to 
>be faster than yours, because it will have to loop through already evaluated 
>ACLs while your code most probably needs to re-evaluate ACLs. (Your code will 
>be faster if the backend sends the attributes in the right order, but I don't 
>see how it could accomplish this).
>
>
>> Disadvantages:
>>         only addresses common read scenarios,
>>         some modify scenarios will not be benefit
>>           (e.g. modify: delete/member/value + add/member/value)
>
>well there is on other scenario that will not benefit:
>
>imagine the following acls:
>
>access to attr=a,c,e,g,i,l by dn="uid=blabla" read;
>access to attr=b,d,f,h,j,k by dn="uid=blubblub" read;
>
>and the object contains all these attributes (returned in the order 
>a,b,c,d,e,f,...)
>
>With your approach you will have to evaluate 11 ACLs, with my approach 2 (and 
>with the original code 24).
>
>Yours,
>Stephan Siano
>
>-- 
>Stephan Siano                           Mail:  Stephan.Siano@suse.de
>SuSE Linux Solutions AG                 Phone: 06196 50951 31
>Mergenthalerallee 45-47                 Fax:   06196 409607
>D-65760 Eschborn