Re: snprintf.c hammering memset() - Mailing list pgsql-hackers

From Andres Freund
Subject Re: snprintf.c hammering memset()
Date
Msg-id 20181001235943.etjqr4lmf2kgxkg3@alap3.anarazel.de
Whole thread Raw
In response to Re: snprintf.c hammering memset()  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: snprintf.c hammering memset()
List pgsql-hackers
Hi,

On 2018-10-01 19:52:40 -0400, Tom Lane wrote:
> Thomas Munro <thomas.munro@enterprisedb.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


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: snprintf.c hammering memset()
Next
From: Tom Lane
Date:
Subject: Re: SerializeParamList vs machines with strict alignment