On 20 December 2015 at 03:06, Andres Freund <and...@anarazel.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: errcheck1.patch.bz2
Description: BZip2 compressed data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to