Thread: [PATCH v1] elog.c: Remove special case which avoided %*s format strings..

..which should no longer be needed since it was a performance hack for specific
platform snprintf, which are no longer used.

On Sun, Aug 02, 2020 at 11:59:48PM -0500, Justin Pryzby wrote:
> ..which should no longer be needed since it was a performance hack for specific
> platform snprintf, which are no longer used.

Did you check if our implementation of src/port/snprintf.c makes %*s
much slower than %s or not?  FWIW, I have just run a small test on my
laptop, and running 100M calls of snprintf() with "%s" in a tight loop
takes 2.7s, with "%*s" and a padding of 0 it takes 4.2s.  So this test
tells that we are far from something that's substantially slower, and
to simplify the code your change makes sense.  Still, there could be a
point in keeping this optimization, but fix the comment to remove the
platform-dependent part of it.  Any thoughts?
--
Michael

On Tue, 4 Aug 2020 at 19:36, Michael Paquier <michael@paquier.xyz> wrote:
> Did you check if our implementation of src/port/snprintf.c makes %*s
> much slower than %s or not?  FWIW, I have just run a small test on my
> laptop, and running 100M calls of snprintf() with "%s" in a tight loop
> takes 2.7s, with "%*s" and a padding of 0 it takes 4.2s.  So this test
> tells that we are far from something that's substantially slower, and
> to simplify the code your change makes sense.  Still, there could be a
> point in keeping this optimization, but fix the comment to remove the
> platform-dependent part of it.  Any thoughts?

It's not just converting "%s" to "%*s", it's sometimes changing a
appendStringInfoString() call to appendStringInfo(). It's hard to
imagine the formatting version could ever be as fast as
appendStringInfo().

FWIW, the tests I did to check this when initially working on it are
in [1].  Justin, it would be good if you could verify you're making as
bad as what's mentioned on that thread again.

David

[1]
https://www.postgresql.org/message-id/flat/20130924165104.GQ4832%40eldon.alvh.no-ip.org#4e8a716ff0bde1e950fe7ddca1d75454

On Tue, Aug 04, 2020 at 09:06:16PM +1200, David Rowley wrote:
> FWIW, the tests I did to check this when initially working on it are
> in [1].  Justin, it would be good if you could verify you're making as
> bad as what's mentioned on that thread again.

Ouch.  Thanks for the reference.  Indeed it looks that it would hurt
even with just a simple PL function.
--
Michael