Re: Keep elog(ERROR) and ereport(ERROR) calls in the cold path

On Sat, 5 Sep 2020 at 08:36, Peter Eisentraut
<peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
> Something based on the v4 patch makes sense.

Thanks for having a look at this.

> I would add DEBUG1 back into the conditional, like
>
> if (__builtin_constant_p(elevel) && ((elevel) >= ERROR || (elevel) <=
> DEBUG1) ? \

hmm, but surely we don't want to move all code that's in the same
branch as an elog(DEBUG1) call into a cold area.

With elog(ERROR) we generally have the form:

if (<some condition we hope never to see>)
elog(ERROR, "something bad happened");

In this case, we'd quite like for the compiler to put code relating to
the elog in some cold area of the binary.

With DEBUG we often have the form:

<do normal stuff>

elog(DEBUG1, "some interesting information");

<do normal stuff>

I don't think we'd want to move the <do normal stuff> into a cold area.

The v3 patch just put an unlikely() around the errstart() call if the
level was <= DEBUG1. That just to move the code that's inside the if
(errstart(...)) in the macro into a cold area.

> Also, for the __has_attribute handling, I'd prefer the style that Andres
> illustrated earlier, using:
>
> #ifndef __has_attribute
> #define __has_attribute(attribute) 0
> #endif

Yip, for sure. That way is much nicer.

David