Re: Keep elog(ERROR) and ereport(ERROR) calls in the cold path

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