Re: [patch] Cleanup: don't call free(NULL) unnecessarily

["Carlos O'Donell" <carlos at redhat dot com>]

On 03/11/2013 12:55 PM, Roland McGrath wrote:
> Since free (NULL) is thoroughly well-specified as a no-op, it never
> qualifies as a "cleanup" to add code to avoid it.  If anything, it's an
> optimization.  Like all claims of optimization, it needs exploration of
> the trade-offs involved and justification for the change.

The use of the word "cleanup" depends on the scope with which you consider
the statement in question. In this case we have logically identical code
already checking for free (NULL) and avoiding it.

Though one might argue we should remove all free (NULL) checks to reduce
I-cache pressure for the fast-path.

> It was past policy never to add tests before free calls.  The rationale
> for that is that free (NULL) is cheap enough as a no-op and the cases
> where it would actually be called are not in hot paths.  

What I need to know as a maintainer is the probability of free (NULL)
being called in the "average use of the library" because this is
the weighting that I will use to determine if the trade off of between
two sequences of code.

How do I determine that?

Do I stick a systemtap probe into vfprint, run the testsuite and look
at the results?

I'd like a deterministic way of figuring this out.

I doubt any human can look at the glibc code base and easily point out
what is or is not a hot-path in some of the more complex functions.

> If they're not
> hot paths, then there is no justification for an optimization change.
> In fact, adding any instructions at all is should always be presumed to
> be a pessimization due to general I-cache pressure.  So adding a test
> must be justified on a specific performance-analysis basis as an
> optimization, and weighed against the maintenance burden of making the
> source code larger and more complex.

That makes sense in general.

Cheers,
Carlos.