On Thu, Oct 20, 2016 at 06:14:28PM +0100, Greg Stark wrote: > On Oct 20, 2016 5:27 PM, "Noah Misch" <n...@leadboat.com> wrote: > > On Wed, Oct 19, 2016 at 11:08:39AM +0100, Greg Stark wrote: > > > The MEMPOOL_FREE doesn't take any size argument and mcxt.c doesn't > > > have convenient access to a size argument. It could call the > > > GetChunkSpace method but that will include the allocation overhead and > > > > That is indeed a problem for making VALGRIND_MEMPOOL_FREE() an alias of > > VALGRIND_MAKE_MEM_NOACCESS() under ASAN as I suggested. Calling > > GetMemoryChunkSpace() in the macro would cause memdebug.h to embed an > > assumption of mcxt.c, which is messy. Including the allocation overhead is > > fine, though. > > I think the way out is to simply have aset.c make the memory > undefined/noaccess even if it's redundant under valgrind. It's a bit > unfortunate that the macros would have different semantics under the two. > If it's too confusing or we're worried about the performance overhead we > could make a MAKE_MEM_{UNDEFINED,NOACCESS}_IF_NO_MEMPOOL() but I don't > think it's worth it myself.
I don't expect much performance overhead. When I last benchmarked Valgrind Memcheck of "make installcheck", a !USE_VALGRIND build (no client requests at all) saved about 5% of runtime. A single new client request should be cheap enough. (Marking otherwise-redundant calls may still be good, though.) > There are a couple build oddities both with gcc and clang before I can > commit anything though. And I can't test valgrind to be sure the redundant > calls aren't causing a problem. When you submit your patch to a CommitFest, mention that you're blocked on having a reviewer who can test Valgrind. Many reviewers can help. Thanks, nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers