Re: [HACKERS] An unlikely() experiment - Mailing list pgsql-hackers

From Andres Freund
Subject Re: [HACKERS] An unlikely() experiment
Date
Msg-id 20171030094449.ffqhvt5n623zvyja@alap3.anarazel.de
Whole thread Raw
In response to Re: [HACKERS] An unlikely() experiment  (David Rowley <david.rowley@2ndquadrant.com>)
Responses Re: [HACKERS] An unlikely() experiment
List pgsql-hackers
On 2017-10-30 22:39:01 +1300, David Rowley wrote:
> On 30 October 2017 at 22:34, Andres Freund <andres@anarazel.de> wrote:
> > Hi,
> >
> > On 2015-12-20 02:49:13 +1300, David Rowley wrote:
> >> 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.
> >
> > I just was thinking about this, and it turns out that
> > __attribute__((cold)) does what we need here. We could just mark
> > elog_start() and errstart() as cold, and afaict that should do the
> > trick.
> 
> I had actually been thinking about this again today. I had previously
> not thought  __attribute__((cold)) would be a good idea since if we
> mark elog_start() with that, then code paths with elog(NOTICE) and
> elog(LOG) not to mention DEBUG would be marked as cold.

That's an issue, true. Although I'm not convinced it's particularly
fatal - if you're ok with emitting debugging output in places it's
probably not the hottest codepath in the first place. And profile guided
optimization would overwrite hot/cold.

But:

> Today I was thinking, to get around that issue, we might be able to
> generate another thin wrapper around elog_start() and mark that as
> __attribute__((cold)) and fudge the macro a bit to call that function
> instead if it can detect a compile time const level >= ERROR. I've not
> looked at the code again to remind myself if that would be possible.

Yes, that's what I was thinking too. Add a elog_fatal() wrapping
elog_finish(), and move the if (__builtin_constant_p(elevel)) branch a
bit earlier, and that should work.  Similar with errstart_fatal() for
ereport().

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

pgsql-hackers by date:

Previous
From: Chris Travers
Date:
Subject: Re: [HACKERS] WIP: Restricting pg_rewind to data/wal dirs
Next
From: Nikolay Shaplov
Date:
Subject: Re: [HACKERS] [PATCH] Move all am-related reloption code into src/backend/access/[am-name] and get rid of relopt_kind for custom AM