Hi,
On 2024-06-30 16:47:02 +0200, Matthias van de Meent wrote:
> 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).
I don't think this could be fairly described as penalizing use of the hooks.
likely()/unlikely() isn't the same as __attribute__((cold)).
See https://godbolt.org/z/qdnz65ez8 - only the version using
__attribute__((cold)) gets some code put into a separate section.
What's the possible argument for saying that the generated code should be
optimized for the presence of hooks?
Note that I'm not saying that we ought to use likely() here (nor the
opposite), just that this argument for not doing so doesn't seem to hold
water.
> If this code is hot enough, then CPU branch prediction will likely
> correctly predict this branch correctly after a small amount of time
> (as hook null-ness is generally approximately constant for the
> duration of a backend lifetime);
IMO that's not quite right. The branch predictor has a limited
capacity. Because postgres executes a lot of code it frequently executes
"outer" code without benefit of the branch predictor, just due to the amount
of code that was executed - even if the outer code matters for performance. In
which case the static branch prediction can matter (i.e. backward branches are
assumed to be taken, forward branches not).
> while if this code is cold, the benefit is not worth the additional binary
> size overhead of the out-of-lined code section.
There wouldn't be any such overhead, afaict.
Greetings,
Andres Freund