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

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]/messages/by-id/20130924165104.GQ4832@eldon.alvh.no-ip.org. Justin, it would be good if you could verify you're making as
bad as what's mentioned on that thread again.

David

[1]: /messages/by-id/20130924165104.GQ4832@eldon.alvh.no-ip.org

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