On 30 October 2017 at 22:44, Andres Freund <andres@anarazel.de> wrote:
> On 2017-10-30 22:39:01 +1300, David Rowley wrote:
>> Today I was thinking, to get around that issue, we might be able to
>> generate another thin wrapper around elog_start() and mark that as
>> __attribute__((cold)) and fudge the macro a bit to call that function
>> instead if it can detect a compile time const level >= ERROR. I've not
>> looked at the code again to remind myself if that would be possible.
>
> Yes, that's what I was thinking too. Add a elog_fatal() wrapping
> elog_finish(), and move the if (__builtin_constant_p(elevel)) branch a
> bit earlier, and that should work. Similar with errstart_fatal() for
> ereport().
This may have been too good to be true. I can't seem to get it to work
and I think it's because the function is inside the do{}while(0) and
the if (__builtin_constant_p(elevel) && (elevel) >= ERROR) branch,
which appears to mean that:
"The paths leading to call of cold functions within code are marked as
unlikely by the branch prediction mechanism" [1]
is not the path that the macro is in in the calling function, like we
might have hoped.
I can get the assembly to change if I put an unlikely() around the
condition or if I go and vandalize the macro to become:
#define elog(elevel, ...) \
elog_start_error(__FILE__, __LINE__, PG_FUNCNAME_MACRO)
[1] https://gcc.gnu.org/onlinedocs/gcc-4.8.5/gcc/Function-Attributes.html
-- David Rowley http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers