On Fri, 11 Sep 2020 at 02:01, Peter Eisentraut <peter.eisentr...@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
elog_ereport_attribute_cold_v4.patch
Description: Binary data
tpch_scale5_elog_ereport_cold_v4_vs_master.ods
Description: application/vnd.oasis.opendocument.spreadsheet
