On Wed, Sep 25, 2013 at 6:25 AM, Andres Freund <and...@2ndquadrant.com>wrote:
> On 2013-09-24 19:56:32 +0200, Andres Freund wrote: > > On 2013-09-24 13:51:04 -0300, Alvaro Herrera wrote: > > > David Rowley escribió: > > > > > > > I do see a 15-18% slow down with the patched version, so perhaps > I'll need > > > > to look to see if I can speed it up a bit, although I do feel this > > > > benchmark is not quite a normal workload. > > > > > > Ouch. That's certainly way too much. Is the compiler inlining > > > process_log_prefix_padding()? If not, does it do it if you add > "inline" > > > to it? That might speed up things a bit. If that's not enough, maybe > > > you need some way to return to the original coding for the case where > no > > > padding is set in front of each option. > > > > From a very short look without actually running it I'd guess the issue > > is all the $* things you're now passing to do appendStringInfo (which > > passes them off to vsnprintf). > > How does it look without that? > > That's maybe misunderstandable, what I mean is to have an if (padding > > 0) around the the changed appendStringInfo invocations and use the old > ones otherwise. > > Yeah I had the same idea to try that next. I suspect that's where the slow down is rather than the processing of the padding. I'm thinking these small tweaks are going to make the code a bit ugly, but I agree about the 15-18% slowdown is a no go. The only other thing apart from checking if padding > 0 is to check if the char after the % is > '9', in that case it can't be formatting as we're only allowing '-' and '0' to '9'. Although I think that's a bit hackish, but perhaps it is acceptable if it helps narrow the performance gap. More benchmarks to follow soon. David > Greetings, > > Andres Freund > > -- > Andres Freund http://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Training & Services >
