Bonjour Michaël,
>> TBH, my recommendation would be to drop *all* of these likely()
>> and unlikely() calls. What evidence have you got that those are
>> meaningfully improving the quality of the generated code? And if
>> they're buried inside macros, they certainly aren't doing anything
>> useful in terms of documenting the code.
>
> Yes. I am wondering if we should not rework this part of the logging
> with something like the attached. My 2c, thoughts welcome.
ISTM that the intent is to minimise the performance impact of ignored
pg_log calls, especially when under debug where it is most likely to be
the case AND that they may be in critical places.
Compared to dealing with the level inside the call, the use of the level
variable avoids a call-test-return cycle in this case, and the unlikely
should help the compiler reorder instructions so that no actual branch is
taken under the common case.
So I think that the current situation is a good thing at least for debug.
For other levels, they are on by default AND would not be placed at
critical performance points, so the whole effort of avoiding the call are
moot.
I agree with Tom that __pg_log_level variable name violates usages.
ISTM that switching the variable to explicitely global solves the issues,
and that possible the level test can be moved to inside the function for
all but the debug level. See attached which reprises some of your idea,
but keep the outside filtering for the debug level.
--
Fabien.
