Hi,
On 2018-10-01 20:19:16 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2018-10-01 19:52:40 -0400, Tom Lane wrote:
> >> 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?
>
> That patch takes the memset out of the main line, but it'd still be
> a performance problem for formats using argument reordering; and the
> stack-space concern would remain the same.
What I mean is that it shouldn't be that hard to only zero out the
portions of the array that are actually used, and thus could refrain
from introducing the limit.
> > 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?
>
> argtypes is only a small part of the stack-space issue, there's also
> argvalues which is (at least) twice as big. I don't think second-guessing
> the compiler about the most efficient representation for an enum is
> going to buy us much here.
Sure, but argvalues isn't zeroed out. As long as the memory on the
stack isn't used (as is the case for most of arvalues elements), the
size of the stack allocation doesn't have a significant efficiency
impact - it's still just an %rsp adjustment. If we're going to continue
to zero out argtypes, it's sure going to be better to zero out 16 rather
than 64bytes (after limiting to 16 args).
Greetings,
Andres Freund