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 |
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 |