On Fri, Jan 25, 2013 at 6:56 PM, Alex Rousskov <[email protected]> wrote: > On 01/24/2013 07:27 PM, Amos Jeffries wrote: >> On 24/01/2013 11:36 p.m., Kinkie wrote: >>> On Thu, Jan 24, 2013 at 11:28 AM, Amos Jeffries wrote: >>>> On 24/01/2013 5:56 a.m., Kinkie wrote: >>>>> Hi Amos, >>>>> what is the best way to signal in a mempools client that the >>>>> constructor is complete and that thus zeroing is not necessary? It's >>>>> not immediately evident from looking at the source.. >>>>> It seems that this is only done in mem.cc for string pools. >>>> >>>> memDataInit() has an optional bool flag. Set it to false. >>> .. I still can't fully understand how it relates to the usual >>> MEMPROXY_CLASS/MEMPROXY_CLASS_INLINE, and I can't find any good >>> example. >>> >>> >>> -- >>> /kinkie >> >> I think I found the answer. And its no good... >> >> The objects defined with MEMPROXY_CLASS() macro are all using >> MemAllocatorProxy::getAllocator() in lib/MemPools.cc to late-initialize >> their pools. There is no zero flag being passed around in any of those >> macros or their code paths. >> >> So far as I can see to make it optional we will need to make either a >> template or a new macro which accepts the flag and saves it for use when >> MemAllocatorProxy::getAllocator().create(...) is done. The doZero() >> setter needs to be called from the same if() condition right after >> MemAllocatorProxy::getAllocator().create(). >> >> So: >> 1) >> - update the macro and roll it out in one step defaulting as (true) >> everywhere >> - then set it to false as things get verified. >> - then eventually remove it again when we have a fully no-zero state >> for all classes. > > I think this is the best option, especially if you trust Coverity to > eventually find all initialization problems. It comes with virtually no > initial risk and results in very few code changes. > > >> 2) >> - add a second macro which sets it to true. >> - roll out a conversion to the new maro > > OK, but I think #1 is better because #1 results in fewer code changes. I > may change my opinion if Kinkie's tests prove that fewer doZero buffers > result in significantly better performance, justifying per-class, > incremental changes.
I'd favor 2 as it's an incremental change, but maybe we have a way to measure:I can test-enable this and run a polygraph to evaluate the impact. It'd also help test correctness. What do you think?
