Many of you might know of GCC's __builtin_expect() and that it allows us to
tell the compiler which code branches are more likely to occur. The
documentation on this does warn that normally it's a bad idea to use this
"as programmers are notoriously bad at predicting how their programs
actually perform" [1].  So I choose to ignore that, and give it a try
anyway... elog(ERROR) for example, surely that branch should always be in a
cold path? ... ereport() too perhaps?

Anyway, I knocked up a patch which went an added an errcheck() macro which
is defined the same as unlikely() is, and I stuck these around just the
"if" conditions for tests before elog(ERROR) calls, such as:

if (errcheck(newoff == InvalidOffsetNumber))
elog(ERROR, "failed to add BRIN tuple to new page");

gcc version 5.2.1 on i7-4712HQ

pgbench -M prepared -T 60 -S
Master: 12246 tps
Patched 13132 tsp (107%)

pgbench -M prepared -T 60 -S -c 8 -j 8
Master: 122982 tps
Patched: 129318 tps (105%)

I thought this was quite interesting.

Ok, so perhaps it's not that likely that we'd want to litter these
errcheck()s around everywhere, so I added a bit more logging as I thought
it's probably just a handful of calls that are making this improvement. I
changed the macro to call a function to log the __FILE__ and __LINE__, then
I loaded the results into Postgres, aggregated by file and line and sorted
by count(*) desc. Here's a sample of them (against bbbd807):

       file       | line | count
 fmgr.c           | 1326 | 158756
 mcxt.c           |  902 | 147184
 mcxt.c           |  759 | 113162
 lwlock.c         | 1545 |  67585
 lwlock.c         |  938 |  67578
 stringinfo.c     |  253 |  55364
 mcxt.c           |  831 |  47984
 fmgr.c           | 1030 |  36271
 syscache.c       |  978 |  28182
 dynahash.c       |  981 |  22721
 dynahash.c       | 1665 |  19994
 mcxt.c           |  931 |  18594

Perhaps it would be worth doing something with the hottest ones maybe?

Alternatively, if there was some way to mark the path as cold from within
the path, rather than from the if() condition before the path, then we
could perhaps do something in the elog() macro instead. I just couldn't
figure out a way to do this.

Does anyone have any thoughts on this?


 David Rowley         
 PostgreSQL Development, 24x7 Support, Training & Services

Reply via email to