Re: 20080207-squid3-nonzero-buffers.diff

On Thu, 2008-02-07 at 09:13 +0900, Adrian Chadd wrote:
> Updated patch:
>
> http://www.creative.net.au/diffs/20080207-squid3-nonzero-buffers-2.diff
>
> doZeroOnPush can't be private as it then seems to be inaccessible from
> MemPool::push().

Ouch. I did not realize the two ended up in different classes.

doZeroOnPush has nothing to do with allocation and is not even used by
MemAllocator, but it has to be there and not in MemPool because
memPoolCreate macro returns MemImplementingAllocator and not a MemPool.

Too much mess to untangle for this simple patch :-(. Let's commit the
second version as is.

Thank you,

Alex.

> On Wed, Feb 06, 2008, Alex Rousskov wrote:
> > Adrian,
> >
> > A few comments regarding the nonzero patch you posted on IRC:
> > http://www.creative.net.au/diffs/20080207-squid3-nonzero-buffers.diff
> >
> > First of all, it is a move in the right direction, of course. If you
> > verified that it works, it can be committed as is. Here are a few
> > nitpicks since you asked for comments:
> >
> > - Replace "dontZeroMe()" with "zeroOnPush(bool doIt)" because negative
> > names are confusing and because we are not zeroing the object ("this" or
> > "me"), but the memory we push into it.
> >
> > - Remove virtual from dontZeroMe(). Makes folks look hard for
> > extensions, etc.
> >
> > - Make zeroBuffer private. Alternatively, make it public and remove the
> > set method. Protected data members cause weird C++ problems,
> > unfortunately.
> >
> > - Rename zeroBuffer to doZeroOnPush so that the association with the set
> > method is clear and the purpose is more precise.
> >
> > - Remove memDataNonZero() if possible. It do nothing new and we need
> > fewer globals.
> >
> > - Consider adding boolean aZeroOnPush parameter to memDataInit(), with
> > false as the default value.
> >
> > Thank you,
> >
> > Alex.
> >
>
Received on Wed Feb 06 2008 - 20:24:49 MST