Re: [squid-dev] RFC: use clang-format?

[Alex Rousskov]

On 4/4/20 4:54 AM, Francesco Chemolli wrote:

>    astyle is a bit of PITA, maybe we can replace it with clang-format?
> It seems to me it has more power and flexibility, and its config could
> be stored in the source tree itself.

I would expect clang-format to be overall better than astyle, but
somebody would need to reformat Squid using a reasonable clang
configuration, share the diff, and honestly analyze specific advantages
and disadvantages of using clang before an informed decision can be made.

BTW, astyle configuration can also be stored in the repository AFAICT:
http://astyle.sourceforge.net/astyle.html#_Option_Files

Alex.
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

Re: [squid-dev] RFC: cacheMatchAcl

[Alex Rousskov]

On 4/4/20 2:49 AM, Francesco Chemolli wrote:

> I’m mostly done in a PR replacing the dlink with a std::list but without
> changing the overall design. It does kill a few tens of lines of code
> and is clearer to read tho.

Please note that std::list is usually a lot more expensive than dlink
(i.e., an invasive list) as far as performance is concerned because
std::list does one extra memory allocation for every list element. In
performance-sensitive cases, we should not replace dlink with std::list.


HTH,

Alex.


> On Sat, 4 Apr 2020 at 07:41, Amos Jeffries wrote:
> 
> On 4/04/20 3:34 am, Alex Rousskov wrote:
> > On 4/3/20 7:25 AM, Francesco Chemolli wrote:
> >
> >>   I'm looking at places where to improve things a bit, and I stumbled
> >> across cacheMatchAcl . It tries hard to be generic, but it is
> only ever
> >> used in ACLProxyAuth::matchProxyAuth . Would it make sense to
> just have
> >> a specialised cache for proxyauth?
> >
> > I wonder whether proxy_auth is special in this context:
> >
> > 1. Is proxy_auth cache radically different from other ACL caches
> such as
> > external ACL cache? Or did we just not bother unifying the code
> > supporting these two caches?
> >
> 
> Pretty much yes, we have not done the legwork. Almost every component in
> Squid which deals with externally provided state has some form of ad-hoc
> cache. If we are lucky the use a hash or dlink. One at least uses splay
> (ouch).
> 
> 
> One of my background projects in the effort to empty the PR queue this
> year is to implement a proper CLP Map - specifically for PR 30 instead
> of the LruMap disagreement blocking it. That would be a good container
> to use for all these small state data caches all over Squid - keyed
> access with a dual TTL and LFU (fading) removal mechanism.
> 
> If this ACL cache is not causing issues already we can wait until that
> gets submitted for review.
> 
> 
> > 2. Do some other ACLs cache nothing just because we did not have
> enough
> > time to add the corresponding caching support? Or do proxy_auth and
> > external ACL poses some unique properties that no other ACL
> already has
> > or likely to have in the foreseeable future?
> 
> The only thing special is that cache they use is exclusively accessed by
> them.
> 
> IDENT, ASN, DNS based ACLs also use caches. But those are a bit detached
> from the ACL code itself (eg fqdncache) since other code sometimes
> accesses the cache directly for other uses.
> 
> 
> Amos
> ___
> squid-dev mailing list
> squid-dev@lists.squid-cache.org 
> http://lists.squid-cache.org/listinfo/squid-dev
> 
> -- 
> @mobile
> 
> ___
> squid-dev mailing list
> squid-dev@lists.squid-cache.org
> http://lists.squid-cache.org/listinfo/squid-dev
> 

___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

[squid-dev] RFC: use clang-format?

[Francesco Chemolli]

Hi all,
   astyle is a bit of PITA, maybe we can replace it with clang-format?
It seems to me it has more power and flexibility, and its config could be
stored in the source tree itself.
What do you think?

-- 
Francesco
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

Re: [squid-dev] RFC: cacheMatchAcl

[Francesco Chemolli]

It was quite fast really as my PR doesn’t address the fundamental design
issues nor much of the coding style. A quick win could be to move from a
list to an unordered_map, it’s a quite trivial cache in the end. There’s a
lot of machinery in there for such a simple data structure :/

On Sat, 4 Apr 2020 at 08:15, Amos Jeffries  wrote:

> On 4/04/20 7:49 pm, Francesco Chemolli wrote:
> > I am not sure about what you recommend to do here.
> > This cache is IMO over complicated and it breaks layering.
> > I’m mostly done in a PR replacing the dlink with a std::list but without
> > changing the overall design. It does kill a few tens of lines of code
> > and is clearer to read tho.
> >
>
> Well, if you have already done the work it migth as well finish up. I
> was thinking you had just done the investigation and not started
> refactoring yet.
>
> Amos
>
-- 
@mobile
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

Re: [squid-dev] RFC: cacheMatchAcl

[Amos Jeffries]

On 4/04/20 7:49 pm, Francesco Chemolli wrote:
> I am not sure about what you recommend to do here.
> This cache is IMO over complicated and it breaks layering.
> I’m mostly done in a PR replacing the dlink with a std::list but without
> changing the overall design. It does kill a few tens of lines of code
> and is clearer to read tho.
> 

Well, if you have already done the work it migth as well finish up. I
was thinking you had just done the investigation and not started
refactoring yet.

Amos
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

Re: [squid-dev] RFC: cacheMatchAcl

[Francesco Chemolli]

I am not sure about what you recommend to do here.
This cache is IMO over complicated and it breaks layering.
I’m mostly done in a PR replacing the dlink with a std::list but without
changing the overall design. It does kill a few tens of lines of code and
is clearer to read tho.

On Sat, 4 Apr 2020 at 07:41, Amos Jeffries  wrote:

> On 4/04/20 3:34 am, Alex Rousskov wrote:
> > On 4/3/20 7:25 AM, Francesco Chemolli wrote:
> >
> >>   I'm looking at places where to improve things a bit, and I stumbled
> >> across cacheMatchAcl . It tries hard to be generic, but it is only ever
> >> used in ACLProxyAuth::matchProxyAuth . Would it make sense to just have
> >> a specialised cache for proxyauth?
> >
> > I wonder whether proxy_auth is special in this context:
> >
> > 1. Is proxy_auth cache radically different from other ACL caches such as
> > external ACL cache? Or did we just not bother unifying the code
> > supporting these two caches?
> >
>
> Pretty much yes, we have not done the legwork. Almost every component in
> Squid which deals with externally provided state has some form of ad-hoc
> cache. If we are lucky the use a hash or dlink. One at least uses splay
> (ouch).
>
>
> One of my background projects in the effort to empty the PR queue this
> year is to implement a proper CLP Map - specifically for PR 30 instead
> of the LruMap disagreement blocking it. That would be a good container
> to use for all these small state data caches all over Squid - keyed
> access with a dual TTL and LFU (fading) removal mechanism.
>
> If this ACL cache is not causing issues already we can wait until that
> gets submitted for review.
>
>
> > 2. Do some other ACLs cache nothing just because we did not have enough
> > time to add the corresponding caching support? Or do proxy_auth and
> > external ACL poses some unique properties that no other ACL already has
> > or likely to have in the foreseeable future?
>
> The only thing special is that cache they use is exclusively accessed by
> them.
>
> IDENT, ASN, DNS based ACLs also use caches. But those are a bit detached
> from the ACL code itself (eg fqdncache) since other code sometimes
> accesses the cache directly for other uses.
>
>
> Amos
> ___
> squid-dev mailing list
> squid-dev@lists.squid-cache.org
> http://lists.squid-cache.org/listinfo/squid-dev
>
-- 
@mobile
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

Re: [squid-dev] RFC: cacheMatchAcl

[Amos Jeffries]

On 4/04/20 3:34 am, Alex Rousskov wrote:
> On 4/3/20 7:25 AM, Francesco Chemolli wrote:
> 
>>   I'm looking at places where to improve things a bit, and I stumbled
>> across cacheMatchAcl . It tries hard to be generic, but it is only ever
>> used in ACLProxyAuth::matchProxyAuth . Would it make sense to just have
>> a specialised cache for proxyauth?
> 
> I wonder whether proxy_auth is special in this context:
> 
> 1. Is proxy_auth cache radically different from other ACL caches such as
> external ACL cache? Or did we just not bother unifying the code
> supporting these two caches?
> 

Pretty much yes, we have not done the legwork. Almost every component in
Squid which deals with externally provided state has some form of ad-hoc
cache. If we are lucky the use a hash or dlink. One at least uses splay
(ouch).


One of my background projects in the effort to empty the PR queue this
year is to implement a proper CLP Map - specifically for PR 30 instead
of the LruMap disagreement blocking it. That would be a good container
to use for all these small state data caches all over Squid - keyed
access with a dual TTL and LFU (fading) removal mechanism.

If this ACL cache is not causing issues already we can wait until that
gets submitted for review.


> 2. Do some other ACLs cache nothing just because we did not have enough
> time to add the corresponding caching support? Or do proxy_auth and
> external ACL poses some unique properties that no other ACL already has
> or likely to have in the foreseeable future?

The only thing special is that cache they use is exclusively accessed by
them.

IDENT, ASN, DNS based ACLs also use caches. But those are a bit detached
from the ACL code itself (eg fqdncache) since other code sometimes
accesses the cache directly for other uses.


Amos
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev