Skip site navigation (1) Skip section navigation (2)

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

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>,pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)
Date: 2013-01-13 21:06:37
Message-ID: 20130113210637.GB27004@awork2.anarazel.de (view raw or flat)
Thread:
Lists: pgsql-hackers
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


In response to

Responses

pgsql-hackers by date

Next:From: Tom LaneDate: 2013-01-13 21:09:29
Subject: Re: erroneous restore into pg_catalog schema
Previous:From: Dimitri FontaineDate: 2013-01-13 20:50:06
Subject: Re: erroneous restore into pg_catalog schema

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group