[squid-dev] [PATCH] cleanup external_acl_type cache

[Alex Rousskov]

On 04/12/2017 06:08 AM, Adam Majer wrote:
> On 04/12/2017 03:48 AM, Alex Rousskov wrote:
>> BTW, if you do end up removing this intrusive list, please check whether
>> its ExternalACLEntry::lru "anchor" member should be removed as a side
>> effect. Perhaps you already have checked that.

> Or replaced with list::iterator to allow for O(1) list::erase operation.

IMO, storing std::list::iterator would combine some of the worst
features of the two approaches:

* metadata de/allocation overheads of a non-intrusive std::list
* anchor awkwardness/risks/limitations of an intrusive list

One might even be able to formulate a related Rule of Thumb: If all the
list elements have to store their position in their list (for any
reason), then a performance-sensitive code should use an intrusive list.


> If you need an intrusive list container for performance reasons, please
> consider using Boost.Intrusive instead of rolling your own.
> 
>     http://www.boost.org/doc/libs/1_63_0/doc/html/intrusive/list.html

The idea of using Boost for Squid has been discussed and, IIRC, rejected
(for the time being): We have more than enough troubles coping with
standard C++ features. Dealing with another major library (that should
penetrate a lot of Squid code to gain the most benefits) without an
in-house Boost expert is a bad idea, especially when the Squid code
itself is still too messy to allow for neat/safe conversions like that.
C++11 reduces the pressure to use Boost by providing some of the Boost
features.

To avoid misunderstanding, I have nothing but respect for Boost, I am
sure that the overall quality of Boost code is much higher than what we
write, and I hate reinventing the wheel. Unfortunately, the other
factors are more important in this case.


HTH,

Alex.