On Mon, Nov 18, 2013 at 1:01 AM, Marko Kreen <markokr@gmail.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@gmail.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.
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.
