Re: [PATCH] Remove MemPools use of calloc()

On 13/01/2013 6:51 a.m., Alex Rousskov wrote:
> On 01/11/2013 12:34 AM, Amos Jeffries wrote:
>
>> MemPools contains a doZeroOnPush flag to optimize performance by
>> removing use of memset() as chunks are added back into the pool.
> yes, although to avoid misunderstanding I would phrase it differently:
> "If doZeroOnPush is true, the pool zeros chunks added back into the pool".
>
>
>> However, on closer inspection it is clear that the following pop()
>> process to re-use those chunks is never performing memset() anyway.
> Here, I assume you are talking about pools which have zeroOnPush set to
> false.
>
>
>> As
>> such I believe that there is no special need to use calloc() on these
>> particular object types in the first place.
>>
>> The attached patch updates MemPools to use malloc() instead of calloc()
>> on all types with doZeroOnPush set.
> Yes, if "set" means "set to false" :-)
>
>
>> This should increase performance a
>> little, and allows us to remove the anti-pattern by setting doZeroOnPush
>> for more objects as we can verify they are correctly initialized by
>> their constructors.
> Again, I assume "setting doZeroOnPush" means "setting doZeroOnPush to
> false". In your commit message, please use a less misleading terminology.

Yes.

> Please rename doZeroOnPush and friends to doZero or at least document
> that member to indicate that it affects both allocated and pushed data now.
>
> While you are documenting, it may be a good idea to explain that we zero
> on push because we are afraid of broken code using stale/dirty data
> after the memory was returned to the pool. If we were not afraid of
> that, we would zero on allocate() instead!

Will do.

I think we do not need to have that worry anymore with the C++ compiler
and Coverity use-after-free checks. Do you agree?

Updated patch attached for re-review.

Amos