Re: appendPQExpBufferVA vs appendStringInfoVA

On Mon, Nov 18, 2013 at 1:01 AM, Marko Kreen <markokr(at)gmail(dot)com> wrote:

> On Thu, Nov 14, 2013 at 09:33:59PM +1300, David Rowley wrote:
> > On Sun, Nov 3, 2013 at 3:18 AM, David Rowley <dgrowleyml(at)gmail(dot)com>
> wrote:
> > > I'm low on ideas on how to improve things much around here for now, but
> > > for what it's worth, I did create a patch which changes unnecessary
> calls
> > > to appendPQExpBuffer() into calls to appendPQExpBufferStr() similar to
> the
> > > recent one for appendStringInfo and appendStringInfoString.
> > >
> > Attached is a re-based version of this.
>
> It does not apply anymore, could you resend it?
>
>
I've attached a re-based version.

> I am bit suspicious of performance impact of this patch, but think
> that it's still worthwhile as it decreases code style where single
> string argument is given to printf-style function without "%s".
>
>
This thread probably did not explain very will the point of this patch.
All this kicked up from an earlier patch which added for alignment in the
log_line_prefix GUC. After some benchmarks were done on the proposed patch
for that, it was discovered that replacing appendStringInfoString with
appendStringInfo gave a big enough slowdown to matter in high volume
logging scenarios. That patch was revised and the appendStringInfo()'s were
only used when they were really needed and performance increased again.

I then ran a few benchmarks seen here:
http://www.postgresql.org/message-id/CAApHDvp2uLKPuHJnCkonBGG2VXPvxoLOPzhrGXBS-M0r0v4wSA@mail.gmail.com

To compare appendStringInfo(si, "%s", str); with appendStringinfoString(a,
str); and appendStringInfo(si, str);

The conclusion to those benchmarks were that appendStringInfoString() was
the best function to use when no formatting was required, so I submitted a
patch which replaced appendStringInfo() with appendStringInfoString() where
that was possible and that was accepted.

appendPQExpBuffer() and appendPQExpBufferStr are the front end versions of
appendStringInfo, so I spent an hour or so replacing these calls too as it
should show a similar speedup, though in this case likely the performance
is less critical, my thinking was more along the lines of, "it increases
performance a little bit with a total of 0 increase in code complexity".

The findings in the benchmarks in the link above also showed that we might
want to look into turning appendStringInfoString into a macro
around appendBinaryStringInfo() to allow us to skip the strlen() call for
string constants... It's unclear at the moment if this would be a good idea
or much of an improvement, so it was left for something to think about for
the future.

Regards

David Rowley

--
> marko
>
>