[Spice-devel] [PATCH] improve performances comparing image pixels

[Jonathon Jongsma]

ACK, but a couple minor commit log nits

On Thu, 2015-09-03 at 16:48 +0100, Frediano Ziglio wrote:
> This patch contains a bit of small optimizations.
> It avoid booleans operations which could involve branches replacing

booleans -> boolean

> with binary operations (equal/all_ident -> some_differences).

Please update the above line to change "some_differences" to the new
name for this variable. 

> The other optimization avoid the use of ABS. First the way the macro

avoid -> avoids

> was used (with a large expression) was not easy to optimize by the
> compiler.
> Then instead of using ABS a much simpler range check is used so instead
> of (ABS(n) >= k) a ((n) <= -k || (n) >= k) is used. This looks small
> but modern compiler can translate this not in range check in a couple

compiler -> compilers

> of machine instructions (and a single compare).
> 
> Using operf on same samples (using spice-server-replay) and trying 2 run

run -> runs

> I got
> 
> run             1       2
> -------------------------
> before    104441   106267
> after      92387    91083
> 
> So the performance increase is about 13%.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/red_bitmap_utils.h | 41 ++++++++++++++++++++---------------------
>  1 file changed, 20 insertions(+), 21 deletions(-)
> 
> diff --git a/server/red_bitmap_utils.h b/server/red_bitmap_utils.h
> index 1c13ced..df13c81 100644
> --- a/server/red_bitmap_utils.h
> +++ b/server/red_bitmap_utils.h
> @@ -49,6 +49,7 @@
>  #else
>  #define CONTRAST_TH 8
>  #endif
> +#define CONTRASTING(n) ((n) <= -CONTRAST_TH || (n) >= CONTRAST_TH)
>  
> 
>  #define SAMPLE_JUMP 15
> @@ -62,29 +63,27 @@ static const double FNAME(PIX_PAIR_SCORE)[] = {
>  // return 0 - equal, 1 - for contrast, 2 for no contrast (PIX_PAIR_SCORE is defined accordingly)
>  static inline int FNAME(pixelcmp)(PIXEL p1, PIXEL p2)
>  {
> -    int diff = ABS(GET_r(p1) - GET_r(p2));
> -    int equal;
> +    int diff, any_different;
>  
> -    if (diff >= CONTRAST_TH) {
> +    diff = GET_r(p1) - GET_r(p2);
> +    any_different = diff;
> +    if (CONTRASTING(diff)) {
>          return 1;
>      }
> -    equal = !diff;
>  
> -    diff = ABS(GET_g(p1) - GET_g(p2));
> -
> -    if (diff >= CONTRAST_TH) {
> +    diff = GET_g(p1) - GET_g(p2);
> +    any_different |= diff;
> +    if (CONTRASTING(diff)) {
>          return 1;
>      }
>  
> -    equal = equal && !diff;
> -
> -    diff = ABS(GET_b(p1) - GET_b(p2));
> -    if (diff >= CONTRAST_TH) {
> +    diff = GET_b(p1) - GET_b(p2);
> +    any_different |= diff;
> +    if (CONTRASTING(diff)) {
>          return 1;
>      }
> -    equal = equal && !diff;
>  
> -    if (equal) {
> +    if (!any_different) {
>          return 0;
>      } else {
>          return 2;
> @@ -93,22 +92,22 @@ static inline int FNAME(pixelcmp)(PIXEL p1, PIXEL p2)
>  
>  static inline double FNAME(pixels_square_score)(PIXEL *line1, PIXEL *line2)
>  {
> -    double ret = 0.0;
> -    int all_ident = TRUE;
> +    double ret;
> +    int any_different = 0;
>      int cmp_res;
>      cmp_res = FNAME(pixelcmp)(*line1, line1[1]);
> -    all_ident = all_ident && (!cmp_res);
> -    ret += FNAME(PIX_PAIR_SCORE)[cmp_res];
> +    any_different |= cmp_res;
> +    ret  = FNAME(PIX_PAIR_SCORE)[cmp_res];
>      cmp_res = FNAME(pixelcmp)(*line1, *line2);
> -    all_ident = all_ident && (!cmp_res);
> +    any_different |= cmp_res;
>      ret += FNAME(PIX_PAIR_SCORE)[cmp_res];
>      cmp_res = FNAME(pixelcmp)(*line1, line2[1]);
> -    all_ident = all_ident && (!cmp_res);
> +    any_different |= cmp_res;
>      ret += FNAME(PIX_PAIR_SCORE)[cmp_res];
>  
>      // ignore squares where all pixels are identical
> -    if (all_ident) {
> -        ret -= (FNAME(PIX_PAIR_SCORE)[0]) * 3;
> +    if (!any_different) {
> +        ret = 0;
>      }
>  
>      return ret;