Re: [PATCH v2] Do not release entries that may be kept in local memory cache.

Alex Rousskov <rousskov_at_measurement-factory.com> writes:

> On 07/05/2012 11:10 AM, Dmitry Kurochkin wrote:
>
>> Do not release entries that may be kept in local memory cache.
>
>> void
>> StoreEntry::trimMemory(const bool preserveSwappable)
>> {
> ...
>> + if (memoryCachable())
>> + return;
>> +
>
>
> Now we are penalizing the common case of Squid running without any
> caching (mem_cache 0) too much.
>

Indeed.

> I suggest adding StoreController::keepInLocalMemory(e) that will
> correctly check whether memory caching is enabled at all and then call
> e.memoryCachable() to arrive at the final answer. The new method will be
> called from StoreEntry::trimMemory(). Will that work better?
>

Yes, I think it will.

But perhaps keepInLocalMemory() is not the best name given that it will
check both local and shared memory cache? Also, keepInLocalMemory()
sounds like it should make a final decision whether to cache an entry
which would not be true in our case.

How about adding StoreController::memoryCacheEnabled() method? It would
have a clear meaning. StoreEntry::trimMemory() would then check for
(StoreController::memoryCacheEnabled() && StoreEntry::memoryCachable()).
What do you think?

Regards,
  Dmitry

>
> Thank you,
>
> Alex.
Received on Thu Jul 05 2012 - 17:41:20 MDT