Re: An unlikely() experiment

From: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: An unlikely() experiment
Date: 2015-12-20 01:21:14
Message-ID: CAKJS1f_XBgcRP0tuAWzS1OF27P3V0k-bLpm=X3_+ey3KwF1giA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 20 December 2015 at 03:06, Andres Freund <andres(at)anarazel(dot)de> wrote:

> On 2015-12-20 02:49:13 +1300, David Rowley wrote:
> > So I choose to ignore that, and give
>
> it a try anyway... elog(ERROR) for example, surely that branch should
> > always be in a cold path? ... ereport() too perhaps?
>
> Hm, not exactly what you were thinking of, but it'd definitely be
> interesting to add unlikely annotations to debug elogs, in some
> automated manner (__builtin_choose_expr?). I've previously hacked
> elog/ereport to only support >= ERROR, and that resulted in a speedup,
> removing a log of elog(DEBUG*) triggered jumps.
>
>
Good thinking. It would be interesting to see benchmarks with the hacked
version of elog()

To make sure I understand correctly: Before doing that you manually
> added the errcheck()'s manually? At each elog(ERROR) callsite?
>
>
Yeah manually at most call sites. Although I did nothing in cases like;

if (somethinggood)
dosomethinggood();
else
elog(ERROR, ...);

Quite possibly in that case we could use likely(), but there's also cases
where elog() is in the default: in a switch(), or contained in an else in
an if/else if/else block.

I also left out errcheck() in places where the condition called a function
with some side affect. I did this as I also wanted to test a hard coded
false condition to see how much overhead remained. I did nothing with
ereport().

> One way to do this would be to add elog_on() / ereport_on() macros,
> directly containing the error message. Like
> #define elog_on(cond, elevel, ...) \
> do { \
> if (unlikely(cond)) \
> { \
> elog(elevel, __VA_ARGS__) \
> } \
> } while(0)
>

Interesting idea. Would you think that would be something we could do a
complete replace on, or are you thinking just for the hotter code paths?

I've attached the patch this time, it should apply cleanly to bbbd807. I'd
would be good to see some other performance tests on some other hardware.

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

Attachment Content-Type Size
errcheck1.patch.bz2 application/x-bzip2 85.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Oleg Bartunov 2015-12-20 04:46:56 Re: WIP: bloom filter in Hash Joins with batches
Previous Message Tom Lane 2015-12-19 23:36:10 Re: Making tab-complete.c easier to maintain