Re: pgbench - use pg logging capabilities

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pgbench - use pg logging capabilities
Date: 2020-01-10 07:52:17
Message-ID: alpine.DEB.2.21.2001100659560.5474@pseudo
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


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.

Attachment Content-Type Size
pgbench-log-level-2.patch text/x-diff 3.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2020-01-10 08:01:25 Re: Assert failure due to "drop schema pg_temp_3 cascade" for temporary tables and \d+ is not showing any info after drooping temp table schema
Previous Message Amit Kapila 2020-01-10 07:50:58 Re: [HACKERS] Block level parallel vacuum