Re: [MERGE] Branch prediction hints

On 10/13/2010 10:14 AM, Francesco Chemolli wrote:
> Hi all,
> This patch supersedes yesterday's; it impements autoconf-based
> detection of compiler builtins.
>
> The purpose of this feature is to give explicit hints to compilers
> supporting them about the likely outcome of an if() clause; thus
> allowing the compiler to do explicit optimizations (CPU predictor
> hinting, code layout optimizations) based on the predicted outcome.
> The obvious case is the singleton pattern, where the existence check
> will always be true except for the first time.

> +#define likely(x) __builtin_expect(!!(x), 1)
> +#define unlikely(x) __builtin_expect(!!(x), 0)

* Global names should start with a capital letter.

* Check that the new names were not already #defined, just in case.

* The new macros should be used in performance-critical parts of the
code or in frequently used parts. The cache manager singleton
"optimization" provided as an illustration is neither, and so it is an
example of how _not_ to use the macros. If you need an example, consider
optimizing Must and Assert instead.

* This patch should not be committed unless somebody promises to follow
up with changes to use the new macros.

I vote "-0" on this because I expect that using this code will result in

- no measurable performance improvement for a long time;
- some overheads related to explaining and maintaining the macros.

I will change my vote to +1 if somebody demonstrates measurable
performance improvement on a simple Polygraph test or deployed Squid
after using the proposed macros inside Must() and Assert() definitions.

Thank you,

Alex.
Received on Thu Oct 14 2010 - 18:33:40 MDT