Re: appendStringInfoString() micro-opt

Neil Conway <neilc(at)samurai(dot)com> writes:
> This patch replaces a bunch of call sites of appendStringInfo() with
> appendStringInfoString().

I doubt this saves enough cycles to be worth doing, but if it floats
your boat ...

When I'm tempted to make a dubious micro-optimization, I always ask
myself "is it likely that the sum of all machine time saved by this
change will exceed the amount of person-time I am about to put into
making it?" Given the number of places you're talking about touching,
and the fact that I've never seen appendStringInfo placing high on a
profile, I suspect this doesn't pass that test.

I'm not objecting to your doing it, exactly, just suggesting that there
are better things to spend your time on.

> I was tempted to make appendStringInfoString() a macro, since (a) it's
> just one line of code (b) I'd expect plenty of compilers to be smart
> enough to optimize-out a strlen() on a string-literal arg. The
> downside is that it would require that appendStringInfoString()
> evaluate its arguments more than once. Any comments on whether this is
> worth doing?

This I would object to, since it creates a risk of failure if anyone
is incautious enough to write a non-constant argument to
appendStringInfoString. As soon as you factor any future debugging
into the equation, the probability that you've made a net savings of
time drops to nil :-(. You have to have a *very* large payback to
justify putting that kind of booby-trap into the code, and the payback
from this change is not only not large, there's no evidence that it'd
even be measurable.

regards, tom lane