Re: pgbench - use pg logging capabilities

Michaël,

>> So I think that the current situation is a good thing at least for debug.
>
> If you look at some of my messages on other threads, you would likely
> notice that my mood of the day is to not design things which try to
> outsmart a user's expectations :)

I'm not following you.

ISTM that user expectations is that the message is printed when the level
requires it, and that the performance impact is minimal otherwise.

I'm not aiming at anything different.

> So I would stand on the position to just remove those likely/unlikely
> parts if we want this logging to be generic.

It is unclear to me whether your point is about the whole "if", or only
the compiler directive itself (i.e. "likely" and "unlikely").

I'll assume the former. I do not think we should "want" logging to be
generic per se, but only if it makes sense from a performance and feature
point of view.

For the normal case (standard level, no debug), there is basically no
difference because the message is going to be printed anyway: either it is
check+call+work, or call+check+work. Anything is fine. The directive would
help the compiler reorder instructions so that usual case does not inccur
a jump.

For debug messages, things are different: removing the external test &
unlikely would have a detrimental effect on performance when not
debugging, which is most of the time, because you would pay the cost of
evaluating arguments and call/return cycle on each message anyway. That
can be bad if a debug message is place in some critical place.

So the right place of the the debug check is early. Once this is done,
then why not doing that for all other level for consistency? This is the
current situation.

If the check is moved inside the call, then there is a performance benefit
to repeat it for debug, which is a pain because then it would be there
twice in that case, and it creates an exception. The fact that some macro
are simplified is not very useful because this is not really user visible.

So IMHO the current situation is fine, but the __variable name. So ISTM
that the attached is the simplest and more reasonable option to fix this.

>> 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.
>
> My own taste would be to still keep the variable local to logging.c,
> and use a "get"-like routine to be consistent with the "set" part. I
> don't have to be right, let's see where this discussion leads us.

This would defeat the point of avoiding a function call, if a function
call is needed to check whether the other function call is needed:-)

Hence the macro.

> (I mentioned that upthread, but I don't think it is a good idea to
> discuss about a redesign of those routines on a thread about pgbench
> based on $subject. All the main players are here so it likely does
> not matter, but..)

Yep. I hesitated to be the one to do it, and ISTM that the problem is
small enough so that it can be resolved without a new thread. I may be
naïvely wrong:-)

--
Fabien.