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 _______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
