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
