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@2ndquadrant.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