On Sat, Sep 28, 2013 at 9:44 PM, David Rowley <dgrowle...@gmail.com> wrote:
> I did some benchmarking earlier in the week for the new patch which was > just commited to allow formatting in the log_line_prefix string. In version > 0.4 of the patch there was some performance regression as I was doing > appendStringInfo(buf, "%*s", padding, variable); instead of > appendStringInfoString(buf, variable); This regression was fixed in a later > version of the patch by only using appendStringInfo when the padding was 0. > > More details here: > http://www.postgresql.org/message-id/CAApHDvreSGYvtXJvqHcXZL8_tXiKKiFXhQyXgqtnQ5Yo=me...@mail.gmail.com > > Today I started looking through the entire source tree to look for places > where appendStringInfo() could be replaced by appendStringInfoString(), I > now have a patch which is around 100 KB in size which changes a large > number of these instances. > > Example: > > diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c > index 3107f9c..d0dea4f 100644 > --- a/src/backend/utils/misc/guc.c > +++ b/src/backend/utils/misc/guc.c > @@ -6174,7 +6174,7 @@ flatten_set_variable_args(const char *name, List > *args) > A_Const *con; > > if (l != list_head(args)) > - appendStringInfo(&buf, ", "); > + appendStringInfoString(&buf, ", "); > > > I know lots of these places are not really in performance critical parts > of the code, so it's likely not too big a performance gain in most places, > though to be honest I didn't pay a great deal of attention to how hot the > code path might be when I was editing. > > I thought I would post here just to test the water on this type of change. > I personally don't think it makes the code any less readable, but if > performance is not going to be greatly improved then perhaps people would > have objections to code churn. > > I didn't run any benchmarks on the core code, but I did pull out all the > stringinfo functions and write my own test application. I also did a trial > on a new macro which could further improve performance when the string > being appended in a string constant, although I just wrote this to gather > opinions about the idea. It is not currently a part of the patch. > > In my benchmarks I was just appending a 8 char string constant 1000 times > onto a stringinfo, I did this 3000 times in my tests. The results were as > follows: > > Results: > 1. appendStringInfoString in 0.404000 sec > 2. appendStringInfo with %s in 1.118000 sec > 3 .appendStringInfo in 1.300000 sec > 4. appendStringInfoStringConst with in 0.221000 sec > > > You can see that appendStringInfoString() is far faster than > appendStringInfo() this will be down to appendStringInfo using va_args > whereas appendStringInfoString() just does a strlen() then memcpy(). Test 4 > bypasses the strlen() by using sizeof() which of course can only be used > with string constants. > > Actual code: > 1. appendStringInfoString(str, "test1234"); > 2. appendStringInfo(str, "%s", "test1234"); > 3. appendStringInfo(str, "test1234"); > 4. appendStringInfoStringConst(str, "test1234"); > > > The macro for test 4 was as follows: > #define appendStringInfoStringConst(buf, s) appendBinaryStringInfo(buf, > (s), sizeof(s)-1) > > I should say at this point that I ran these benchmarks on windows 7 64bit, > though my tests for the log_line_prefix patch were all on Linux and saw a > similar slowdown on appendStringInfo() vs appendStringInfoString(). > > So let this be the thread to gather opinions on if my 100kb patch which > replaces appendStringInfo with appendStringInfoString where possible would > be welcomed by the community. Also would using > appendStringInfoStringConst() be going 1 step too far? > > Regards > > I've attached a the cleanup patch for this. This one just converts instances of appendStringInfo into appendStringInfoString where appendStringInfo does no formatting or just has the format "%s". > David Rowley >
appendstringinfo_cleanup_v0.1.patch.gz
Description: GNU Zip compressed data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
