Re: appendStringInfoString() micro-opt - Mailing list pgsql-patches

From Tom Lane
Subject Re: appendStringInfoString() micro-opt
Date
Msg-id 28646.1075065897@sss.pgh.pa.us
Whole thread Raw
In response to appendStringInfoString() micro-opt  (Neil Conway <neilc@samurai.com>)
Responses Re: appendStringInfoString() micro-opt
Re: appendStringInfoString() micro-opt
List pgsql-patches
Neil Conway <neilc@samurai.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

pgsql-patches by date:

Previous
From: Kris Jurka
Date:
Subject: contrib/ltree Readme operator name
Next
From: "Magnus Hagander"
Date:
Subject: Win32 signals patch