Re: Linux likely() unlikely() for PostgreSQL

From: Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, wenhui qiu <qiuwenhuifx(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Linux likely() unlikely() for PostgreSQL
Date: 2024-07-04 11:43:46
Message-ID: CAEudQAryc2Voj9cUOT-3ovcPpw8TnCuGd3GjbJ6ScLeWNhv5Xw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Em dom., 30 de jun. de 2024 às 12:24, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> escreveu:

> Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com> writes:
> > On Sun, 30 Jun 2024, 15:56 wenhui qiu, <qiuwenhuifx(at)gmail(dot)com> wrote:
> >> if (unlikely(ExecutorRun_hook)),
>
> > While hooks are generally not installed by default, I would advise
> > against marking the hooks as unlikely, as that would unfairly penalize
> > the performance of extensions that do utilise this hook (or hooks in
> > general when applied to all hooks).
>
> In general, we have a policy of using likely/unlikely very sparingly,
> and only in demonstrably hot code paths. This hook call certainly
> doesn't qualify as hot.
>
> Having said that ... something I've been wondering about is how to
> teach the compiler that error paths are unlikely. Doing this
> across-the-board wouldn't be "sparingly", but I think surely it'd
> result in better code quality on average. This'd be easy enough
> to do in Assert:
>
> #define Assert(condition) \
> do { \
> - if (!(condition)) \
> + if (unlikely(!(condition))) \
> ExceptionalCondition(#condition, __FILE__,
> __LINE__); \
> } while (0)
>
> but on the other hand we don't care that much about micro-optimization
> of assert-enabled builds, so maybe that's not worth the trouble. The
> real win would be if constructs like
>
> if (trouble)
> ereport(ERROR, ...);
>
> could be interpreted as
>
> if (unlikely(trouble))
> ereport(ERROR, ...);
>
Hi Tom.

Let's do it?
But we will need a 1 hour window to apply the patch.
I can generate it in 30 minutes.
The current size is 1.6MB.

All expressions for ERROR, FATAL and PANIC.

if (unlikely(expr))
ereport(ERROR, ...)

Any other expression was left out, such as:

if (expr)
{
ereport(ERROR, ...)
}

This attached version needs manual adjustment.
for cases of:
if (expr) /* comments */

If I apply it, I can adjust it.

best regards,
Ranier Vilela

Attachment Content-Type Size
convert_ereport_expr_to_unlikely_expr.zip application/x-zip-compressed 309.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2024-07-04 12:00:18 Re: Fix a comment on PQcancelErrorMessage
Previous Message Alvaro Herrera 2024-07-04 11:36:40 Re: LogwrtResult contended spinlock