On 2013-01-11 14:01:40 -0500, Tom Lane wrote:
> Andres Freund <andres@2ndquadrant.com> writes:
> > The attached patch:
>
> > * adds configure checks for __VA_ARGS__ and __builtin_unreachable
> > support
> > * provides a pg_unreachable() macro that expands to
> > __builtin_unreachable() or abort()
> > * changes #define elog ... into #define elog(elevel, ...) if varargs are
> > available
>
> Seems like a good thing to do --- will review and commit.
Thanks.
I guess you will catch that anyway, but afaik msvc supports __VA_ARGS__
these days (since v2005 seemingly) and I didn't add HAVE__VA_ARGS to the
respective pg_config.h.win32
> > It does *not* combine elog_start and elog_finish into one function if
> > varargs are available although that brings a rather measurable
> > size/performance benefit.
>
> Since you've apparently already done the measurement: how much?
> It would be a bit tedious to support two different infrastructures for
> elog(), but if it's a big enough win maybe we should.
Imo its pretty definitely a big enough win. So big I have a hard time
believing it can be true without negative effects somewhere else.
Ontop of the patch youve replied to it saves somewhere around 80kb and
between 0.8 (-S -M prepared), 2% (-S -M simple) and 4% (own stuff) I had
lying around and it consistently gives 6%-7% in Pavel's testcase. With
todays HEAD, not with the one before your fix.
I attached the absolutely ugly and unready patch I used for testing.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services