Re: [RFD PATCH 0/3] Free all the memory!

From: Eric Wong <e@80x24.org>
Date: May 17, 2016, 12:08 UTC
Message-ID: <20160517120815.GA15546@dcvr.yhbt.net>
In-Reply-To: <CAPig+cT9UKEcNXvsQsFDrQycKwMJR5bCter76UL-zpkncniPBw@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 10 quoted lines

> On Mon, May 16, 2016 at 11:22 PM, Stefan Beller <sbeller@google.com> wrote:
> > When using automated tools to find memory leaks, it is hard to distinguish
> > between actual leaks and intentional non-cleanups at the end of the program,
> > such that the actual leaks hide in the noise.
> 
> Considering the signal-to-noise ratio mentioned above, the goal seems
> reasonable, but why pollute the code with #ifdef's all over the place
> by making the cleanup conditional? If you're going though the effort
> of plugging all these leaks, it probably makes sense to do them
> unconditionally.

I haven't checked for git, but I suspect we get speedups by not calling free(). I've never needed to profile git, but free() at exit has been a measurable bottleneck in other projects I've worked on. Often, free() was more expensive than *alloc().

In any case, I like constant conditionals in C or inline wrappers macros over CPP #ifdefs littered inside functions:

/* in git-compat-util.h */ #ifdef FREE_ALL_MEMORY static inline void optional_free(void *ptr) {} #else # define FREE_ALL_MEMORY (0) # define optional_free(ptr) free(ptr) #endif

/* inside any function: */
	if (FREE_ALL_MEMORY)
		big_function_which_calls_multiple_frees();

Also Valgrind has suppression files, so code modifications may not be necessary at all.