Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

On 2013-01-13 15:44:58 -0500, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:

> > And if you look at the disassembly of ERROR codepaths:
>
> I think your numbers are being twisted by -fno-omit-frame-pointer.
> What I get, with the __builtin_unreachable version of the macro,
> looks more like

The only difference is that the following two instructions aren't done:
0x00000000007363b0 <+32>: push %rbp
0x00000000007363b1 <+33>: mov %rsp,%rbp

Given that that function barely uses registers (from an amd64 pov at
least), thats notreally surprising.

> MemoryContextAlloc:
> cmpq $1073741823, %rsi
> pushq %rbx
> movq %rsi, %rbx
> ja .L53
> movq 8(%rdi), %rax
> movb $0, 48(%rdi)
> popq %rbx
> movq (%rax), %rax
> jmp *%rax
> .L53:
> movl $__func__.5262, %edx
> movl $576, %esi
> movl $.LC2, %edi
> call elog_start
> movq %rbx, %rdx
> movl $.LC3, %esi
> movl $20, %edi
> xorl %eax, %eax
> call elog_finish
>
> With -fno-omit-frame-pointer it's a little worse, but still not what you
> show --- in particular, for me gcc still pushes the elog calls out of
> the main code path. I don't think that the main path will get any
> better with one elog function instead of two. It could easily get worse.
> On many machines, the single-function version would be worse because of
> needing to use more parameter registers, which would translate into more
> save/restore work in the calling function, which is overhead that would
> likely be paid whether elog actually gets called or not. (As an
> example, in the above code gcc evidently isn't noticing that it doesn't
> need to save/restore rbx so far as the main path is concerned.)
>
> In any case, results from a single micro-benchmark on a single platform
> with a single compiler version (and single set of compiler options)
> don't convince me a whole lot here.

I am not convinced either, I just got curious whether it would be a win
after the __VA_ARGS__ thing made it possible. If the errno problem
didn't exist I would be pretty damn sure its just about always a win,
but the way its now...

We could make elog behave the same as ereport WRT to argument evaluation
but that seems a bit dangerous given it would still be different on some
platforms.

> Basically, the aspects of this that I think are likely to be
> reproducible wins across different platforms are (a) teaching the
> compiler that elog(ERROR) doesn't return, and (b) reducing code size as
> much as possible. The single-function change isn't going to help on
> either ground --- maybe it would have helped on (b) without the errno
> problem, but that's going to destroy any possible code size savings.

Agreed. I am happy to produce an updated patch unless youre already on
it?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services