Re: SBuf review at r9370

On Thu, Feb 26, 2009 at 7:08 PM, Alex Rousskov
<rousskov_at_measurement-factory.com> wrote:
> On 02/26/2009 10:24 AM, Kinkie wrote:
>> On Thu, Feb 26, 2009 at 1:46 AM, Alex Rousskov
>> <rousskov_at_measurement-factory.com> wrote:
>>
>>> http://bazaar.launchpad.net/~kinkie/squid/stringng at r9370.
>>>
>>> * Remove isLiteral and the corresponding code/logic. We might add this
>>> very minor performance optimization as a non-public Blob member once the
>>> code is known to work well. For now, the code is confusing enough
>>> without it.
>>>
>> The Literal thing was actually added to answer to a very specific
>> need. IIRC it was because without it valgrind complained about leaked
>> memory in case of a global static SBuf.
>> If you're sure, I'll remove it.
>>
> I am even more sure now that I know the reason you added it. :-)

I've removed it, and it's causing me to have headaches to no end, due
to interactions with MemPools.
The scenario is this:

I've extended mempools to add more String sizes, and I've offloaded
part of the logic in estimateCapacity there. Basically, what happens
is that estimateCapacity does not try to round to the pool size
anymore, since the pools are better suited to handle that.

BUT MemPools are initialized AFTER global variables, and thus a global
definition such as:
SBuf foo("some initializer");
causes the SBuf to grow()
which calls memAllocString
which tries to locate a pool, and fails because the pools are not yet ready.
(run...)
at program exit()
~SBuf
calls ~MemBlob
which calls memFreeString
which asserts unless by sheer chance the MemBlob has the exact same
size of a pool, and if it does the results are unpredicatable anyways.

Hacking my way around this is painful, and is causing me to touch
things I'd really rather not touch as much (mem.cc)
isLiteral/isImported had the dubious advantage of flagging in the
object themselves whether they were obtained from pools or from some
form of malloc or string constant. I can't really imagine a different
solution which doesn't break layering.

Do you have any suggestion?

>> I agree, with a SLIGHTLY more complex design to match the current
>> standard MemPool String sizes.
>> Stuff like:
>> desired *= constant_growth_factor;
>> if (desired< ShortStringsSize)
>>   return ShortStringsSize;
>> if (desired < MediumStringsSize)
>>   return ShortStringsSize;
>> if (desired < BigStringsSize)
>>   return BigStringsSize;
>> if (desired < 2kBuffersSize)
>>   return BigStringsSize;

Now removed (but not committed due to the above issues), it's better
done in memAllocString anyways.

-- 
    /kinkie