Re: Keep elog(ERROR) and ereport(ERROR) calls in the cold path - Mailing list pgsql-hackers

From David Rowley
Subject Re: Keep elog(ERROR) and ereport(ERROR) calls in the cold path
Date
Msg-id CAApHDvo1Ygmrwf8a_v+jiucVYO8Lb21AzXiXZnSDUN2OYGGCqQ@mail.gmail.com
Whole thread Raw
In response to Re: Keep elog(ERROR) and ereport(ERROR) calls in the cold path  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
Responses Re: Keep elog(ERROR) and ereport(ERROR) calls in the cold path
List pgsql-hackers
On Fri, 11 Sep 2020 at 02:01, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
>
> On 2020-09-06 02:24, David Rowley wrote:
> >> 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.
>
> Yeah, nevermind that.

I've reattached the v4 patch since it just does the >= ERROR case.

> > 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.
>
> That could be useful.  Depends on how much effect it has.

I wonder if it is. I'm having trouble even seeing gains from the ERROR
case and I'm considering dropping this patch due to that.

I ran another scale=5 TPCH benchmark on v4 against f859c2ffa using gcc
9.3. I'm unable to see any gains with this, however, the results were
pretty noisy. I only ran pgbench for 60 seconds per query. I'll likely
need to run that a bit longer. I'll do that tonight.

It would be good if someone else could run some tests on their own
hardware to see if they can see any gains.

David

Attachment

pgsql-hackers by date:

Previous
From: Ashutosh Sharma
Date:
Subject: Re: Logical replication from PG v13 and below to PG v14 (devel version) is not working.
Next
From: David Rowley
Date:
Subject: Re: VACUUM PARALLEL option vs. max_parallel_maintenance_workers