Re: [PATCH] AND and OR ACLs

On 05/02/2013 09:17 AM, Tsantilas Christos wrote:
> On 05/02/2013 06:57 AM, Alex Rousskov wrote:
>> On 05/01/2013 07:28 PM, Amos Jeffries wrote:
>>> On 2/05/2013 10:52 a.m., Alex Rousskov wrote:
>>>> On 12/21/2012 02:05 AM, Amos Jeffries wrote:
>>> Eureka. That would explain why I have not been able to see a regression.
>>> The previous behaviour we would look at teh logs and see a full ACL test
>>> with no re-scan of a part in the middle, nowdays we can clearly see the
>>> line being restarted from scratch. The above would mean that previously
>>> we were overlooking a partial scan from head to some position Foo before
>>> the scan which was seen as being the "whole" scan.
>>>
>>> My comment stands though, the from-line restart is an artifact of your
>>> change. As you say it was from-head. Both situations are bad, with the
>>> current somewhat better than before.

>> Sorry, you lost me here. Both the old and the current code restart from
>> the head (i.e., the beginning) of the current acl_access rule:
>>
>>>>> - const ACLList *node = head;
>>
>> vs.
>>
>>>>> + for (const ACLList *node = head;
>>
>>
>> and do node matches for every node in that rule (if all acls match):
>>
>>>>> - while (node) {
>>>>> - bool nodeMatched = node->matches(this);
>> ...
>>>>> - node = node->next;
>>
>> vs.
>>
>>>>> + for (const ACLList *node = head; node; node = node->next) {
>>>>> + ... resultBeforeAsync = matchNode(*node, fast);

>> I do not know why you saw something in the logs that you do see now, but
>> there is no difference as far as this from-head node rematching aspect
>> is concerned (AFAIK).

> This is how these checks done in squid-2.6 too, so I believe that this
> is by design.
>
> The question is if there are reasons for this, or we can speed up the
> processing avoiding to recheck acls which already had checked.

Just to conclude this discussion: I have removed rechecks. It was
difficult, but I could not stand the idea that a large portion of the
ACL expression tree, with all its growing complexity, may be rescanned
on every slow ACL lookup. I am going to post the new patch shortly.

Thank you,

Alex.
Received on Mon May 13 2013 - 23:19:32 MDT