[squid-dev] [RFC][PATCH] Bug4438 second attempt: give MemBlob its own pools

[Alex Rousskov]

On 03/14/2016 02:11 PM, Kinkie wrote:

>   this second attempt at bug4438 tries a different approach: by giving
> MemBlob its own pools and having a hard initialization dependency in
> MemBlob's allocating function, instead of relying on memAllocString.

Forgive me if I missed how this decision was made earlier, but I do not
understand why we want to duplicate the existing memAllocString()-like
functionality and split a single set of raw memory pools shared by all
callers into two isolated sets?

In other words, how did we get from "SIGSEGV in memFreeString" (i.e.,
bug 4438) to this? If your working (or perhaps already proven!) theory
is that memAllocString() does something wrong, why not fix
memAllocString() instead of duplicating it? I must be missing something
obvious here...


> Known side-effects:
> - allows to tune memblob sizes according to actual use patterns, which
> can be collected via SBuf/MemBlob detailed stats. The currently-set
> sizes follow some light testing I've done.

Why is that a side effect? Could we not "tune memblob sizes according to
actual use patterns" before this change?

Or do you meant that we can tune two separate sets of pools now? If yes,
then you should also list "two separate sets of pools instead of one
shared by all callers" as a side effect -- "more isolated sets" is the
"cost" associated with this ability to tune more sets...


> - as no MemBlob can be initialized before MemPools, there is no need
> to baloon statically-initialized MemBlobs to
> SmallestStringBeforeMemIsInitialized.

If your change guarantees that no MemBlob can be initialized before
MemPools, why cannot a similar change be done to the original code so
that no memAllocString()/etc. caller can get its result before MemPools
are initialized? AFAICT, you have solved the problem that you were
trying to solve, but I do not understand why you had to solve it in a
new location (leaving the old problem unsolved?).


Thank you,

Alex.