Re: snprintf.c hammering memset()

Hi,

On 2018-10-01 19:52:40 -0400, Tom Lane wrote:
> Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> writes:
> > Mateusz Guzik was benchmarking PostgreSQL on FreeBSD investigating the
> > kqueue thread and complained off-list about a lot of calls to memset()
> > of size 256KB from dopr() in our snprintf.c code.
>
> > Yeah, that says:
> > PrintfArgType argtypes[NL_ARGMAX + 2];
> > ...
> > MemSet(argtypes, 0, sizeof(argtypes));
>
> > PrintfArgType is an enum, and we define NL_ARGMAX as 16 if the OS
> > didn't already define it. On FreeBSD 11, NL_ARGMAX was defined as 99
> > in <limits.h>. On FreeBSD 12, it is defined as 65536... ouch. On a
> > Debian box I see it is 4096.
>
> > Is there any reason to use the OS definition here?
>
> Ouch indeed. Quite aside from cycles wasted, that's way more stack than
> we want this to consume. I'm good with forcing this to 16 or so ...
> any objections?

Especially after your performance patch, shouldn't we actually be able
to get rid of that memset entirely?

And if not, shouldn't we be able to reduce the per-element size of
argtypes considerably, by using a uint8 as the base, rather than 4 byte
per element?

> (I wonder if this has anything to do with Andres' performance gripes.)

It probably plays some role, but the profile didn't show it as *too* large
a part.

Greetings,

Andres Freund