Re: [PATCH] AND and OR ACLs

On 12/21/2012 02:05 AM, Amos Jeffries wrote:

> On 21/12/2012 6:28 a.m., Tsantilas Christos wrote:
>> - The new acls converted to Checklists in order to computed.

> I really have a big issue with designing it this way as well. The
> overheads of setting up many Checklists are way out of proportion
> compared to the problem avoided...

> IMO design these ACLs to operate as nodes in the ACL test tree which
> operate on the one Checklist same as all other ACLs.

Hi Amos,

    I am trying to rewrite this patch to address your concerns. I want
to make sure the rewritten patch does not face the same "overhead"
objection. Can you clarify why you think that creating something like a
Checklist (with multiple ACLs) is a lot more expensive than creating a
typical ACL (with multiple values)?

>> My question is, is this any reason why squid is checking from the
>> beginning the access line when an acl needs lookup?
>> Is n't it add a performance overhead?

> It is an artifact of the ACL processing simplification Alex added a
> while back.

I do not think I added any code that rechecks access lines from the
beginning on async lookups! We were restarting the checks from "head"
before r12176, if that is the revision you are implicating:

> - const ACLList *node = head;
> - while (node) {
> - bool nodeMatched = node->matches(this);
> + for (const ACLList *node = head; node; node = node->next) {
> + const NodeMatchingResult resultBeforeAsync = matchNode(*node, fast);

I think these rechecks are expensive, and I have an idea on how we can
avoid them, but doing so would be outside this project scope.

[ r12176 may call matchNode() twice when a non-async code pretends to be
async, but the overhead of doing that is probably minor (especially
compared to rechecking access lines from the beginning on every async
lookup) and the core problem causing those double calls is outside the
ACL code. ]

Thank you,

Alex.
Received on Wed May 01 2013 - 22:52:19 MDT