Re: StringNg review: MemBlob

On Mon, Oct 18, 2010 at 7:56 PM, Alex Rousskov
<rousskov_at_measurement-factory.com> wrote:
> Hi Kinkie,
>
>    Attached is am untested patch with some final polishing changes for
> MemBlob, based on lp:~kinkie/squid/stringng revision 9517. I think the
> MemBlob class should be committed to trunk after these changes are tested
> and polished (I may have missed a few minor problems when moving or renaming
> stuff). The rest of the branch code would need to be synced as well.

revno 9518 covers one of the changes you requested: it turns
MemBlob::calcAllocSize into a nonmember static inline call in
MemBlob.cc.

>
> The patch preamble documents the major changes.
>
> While working on this, I realized that the "MemBlob::size" member is only an
> approximation, and that MemBlob class is probably not usable for efficient
> message body pipelining (BodyPipe and friends). We will probably change
> MemBlob API later to accommodate users other than strings, but that can
> wait. I adjusted the append call to remove the use of pointers which may
> make future changes less disruptive.
>
> Once MemBlob is committed, please consider re-integrating it with memory
> pools. This is the biggest remaining XXX, IMO.

I'd postpone that to after merging SBuf in. The biggest issues I have
met when trying to do that were related to initialization (IIRC):
- static variables can't use mempools
- some nonstatic MemBlobs get allocated before mempools are
initialized. While this can IIRC be detected at initialization time,
what happens at destruction time? Do the blobs need to remember
whether they were pool-allocated or not? Or do mempools handle
gracefully an attempt to free something which was not obtained from a
pool?

The fact that mempools have only been partly converted to c++ doesn't
really help :(

More to come as I check the patch(es).

Thanks for taking the time Alex; I'm really happy to see things progressing.

-- 
    /kinkie