On Sat, 5 Sep 2020 at 08:36, Peter Eisentraut
<peter.eisentraut@2ndquadrant.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