[Date Prev][Date Next] [Chronological] [Thread] [Top]

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

> In reviewing these patches, it looks like acl_check_modlist ought to be
> checking for any value-dependent ACLs as well, but currently isn't. Yes?
> Also, since the caching is only performed on a per-entry basis, the entire
> ACLCache structure looks unnecessary. It also seems to me that nothing is
> gained from making the ACLCacheEntry a doubly linked list, a single link would
> be enough since the list is only traversed in one direction.

I also noted that in the patch there is a lot of unnecessary stuff; it 
looks like provisions for a more comprehensive caching approach (the
structure should be preserved among send_ldap_entry() calls, so that 
checks on different entries by the same operation DN on a similar set 
of attrs can exploit it. I agree on the double linked list.

I was a bit uncomfortable in committing it as is, but I'm out of time
for an adequate review.  Maybe for a while we can live without caching,
although this implies changing the API later.