From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Neil Conway <neilc(at)samurai(dot)com> |
Cc: | PostgreSQL Patches <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: appendStringInfoString() micro-opt |
Date: | 2004-01-25 21:24:57 |
Message-ID: | 28646.1075065897@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-patches |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Magnus Hagander | 2004-01-25 22:09:42 | Win32 signals patch |
Previous Message | Kris Jurka | 2004-01-25 09:21:44 | contrib/ltree Readme operator name |