Out of curiosity, did you measure the performance impact of these
changes? If so, it would be nice to include this in the commit log.
Just a few minor terminology suggestions below
On Fri, 2015-08-21 at 10:57 +0100, Frediano Ziglio wrote:
> This patch contains a bit of small optimizations.
> It avoid booleans operations which could involve branches replacing
"booleans operations" -> boolean operations
> with binary operations (equal/all_ident -> some_differences).
> The other optimization avoid the use of ABS. First the way the macro
> was used (with a large expression) was not easy to optimizeby the
"optimizeby" -> "optimize by"
> 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
> of machine instructions (and a single compare).
>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> 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 7e57a7b..bf82e79 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, some_differents;
Since "different" is an adjective, it sounds strange as a plural. I'd
suggest dropping the 's'. Or maybe changing to "any_different".
>
> - if (diff >= CONTRAST_TH) {
> + diff = GET_r(p1) - GET_r(p2);
> + some_differents = 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);
> + some_differents |= 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);
> + some_differents |= diff;
> + if (CONTRASTING(diff)) {
> return 1;
> }
> - equal = equal && !diff;
>
> - if (equal) {
> + if (!some_differents) {
> 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 some_differents = 0;
Same as previous comment.
> int cmp_res;
> cmp_res = FNAME(pixelcmp)(*line1, line1[1]);
> - all_ident = all_ident && (!cmp_res);
> - ret += FNAME(PIX_PAIR_SCORE)[cmp_res];
> + some_differents |= cmp_res;
> + ret = FNAME(PIX_PAIR_SCORE)[cmp_res];
> cmp_res = FNAME(pixelcmp)(*line1, *line2);
> - all_ident = all_ident && (!cmp_res);
> + some_differents |= cmp_res;
> ret += FNAME(PIX_PAIR_SCORE)[cmp_res];
> cmp_res = FNAME(pixelcmp)(*line1, line2[1]);
> - all_ident = all_ident && (!cmp_res);
> + some_differents |= 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 (!some_differents) {
> + ret = 0;
> }
>
> return ret;
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel