[squid-dev] [PATCH] Non-zeroing mempools

[Amos Jeffries]

On 20/08/2015 7:28 a.m., Kinkie wrote:
> Hi all,
>   the attached patch (a merge request from
> lp:~kinkie/squid/mempools-nozero) changes MEMPROXY_CLASS so that it doesn't
> zero memory before releasing it to requestors. The constructors of all
> classes using that feature were checked, so that they fully initialize all
> data members and thus require no zeroing.
> It also changes Debug::OutStream to use MEMPROXY_CLASS instead of
> new/derelete, and changes constructors to initializer lists whenever
> possible and sensible (aka justified by other changes), same for dropping
> HERE and adopting nullptr.
> It also changes some code in Makefile.am to use make variables in place of
> full paths.
> The code has been run-tested (default config only), build-farm-tested and
> polygraph-tested. It shows a small but noticeable performance improvement
> in performance (of course, numbers are to be taken with a big grain of
> salt).
> 
> When reviewing, please do not focus on formatting issues; if this patch
> passes review I'll run a source-formatting before merging.


* in Makefiles please add the new bits of *_SOURCE lists in alphabetical
order.
 - you have several stub_libmem.cc going after stub_M and stub_S* filenames.


* Please make the initializer lists one member per line.
 - the auto-formatting wont do that for us yet.
 - it greatly helps readability and manual checking for missing or
out-of-order items. We can also then easily add comments to lines
explain odd default values.


* in memset() please use 0 integer value not '\0' to set flags.
 - it helps the compiler know it is safe to optimize up to larger than
8-bit values of 0.


* in MemObject::MemObject() please leave the _reply initialization like
it was so that the HTTMSGLOCK hack is kept visibly associated with the
new operation.


* You add several "// needs mempool-managed zeroing."
 - but you say this patch makes the MEMPROXYC_CLASS macro do zero-ing
already. Those should be reverted.


* src/HttpHeader.cc - revert


* when adding a new constructor ensure the big-5 operators (ctor, dtor,
assign, emplace, emplace-assign) are all present. Even if they use "=
default"
 - looking at StoreMeta, maybe others.


* wordlist missing 'explicit' on the raw-pointer ctor
 - also other big-5 operators


* in test-suite/Makefile.am please try to avoid adding LIBMEM_STUBS.
- DEBUG_SOURCE entries should be enough, and it doesnt matter of extra
stubs get linked to unit tests.


* please do one of these:

 + return the XXX to Debugs.h, but it can move up to explain why
AllocatorProxy.h is used instead of mem/forward.h

 + move the definition of FREE to mem/forward.h, and make anything
including AllocatorProxy.h directly include mem/forward.h instead


Amos