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