On 2013-01-13 15:44:58 -0500, Tom Lane wrote: > Andres Freund <and...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
