[Spice-devel] [PATCH spice-gtk] main: Avoid sqrt in monitor compare

[Christophe de Dinechin]

> On 24 Feb 2017, at 13:26, Pavel Grunt <pgrunt at redhat.com> wrote:
> 
> qsort cares about the sign of the difference and that should stay
> the same
> ---
> src/channel-main.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/src/channel-main.c b/src/channel-main.c
> index cd3dee7..50c29f2 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -17,7 +17,6 @@
> */
> #include "config.h"
> 
> -#include <math.h>
> #include <spice/vd_agent.h>
> #include <glib/gstdio.h>
> #include <glib/gi18n-lib.h>
> @@ -1029,8 +1028,8 @@ static int monitors_cmp(const void *p1, const void *p2, gpointer user_data)
> {
>   const VDAgentMonConfig *m1 = p1;
>   const VDAgentMonConfig *m2 = p2;
> -    double d1 = sqrt(m1->x * m1->x + m1->y * m1->y);
> -    double d2 = sqrt(m2->x * m2->x + m2->y * m2->y);
> +    double d1 = m1->x * m1->x + m1->y * m1->y;
> +    double d2 = m2->x * m2->x + m2->y * m2->y;

If there really are chances for any of the coordinates to go beyond 16-bit, I would write this as:

	long m1x = m1->x;
	long m1y = m1->y;
	long m2x = m2->x;
	long m2y = m2->y;
	long d1 = m1x * m1x + m1y * m1y;
	long d2 = m2x * m2x + m2y * m2y;

The extra cost is zero on x86_64, and it’s giving you the correct result at a lower cost than double.

>   int diff = d1 - d2;
> 
>   return diff == 0 ? (char*)p1 - (char*)p2 : diff;

If we only care about the sign of the difference, I would rewrite this as:

	return diff != 0 ? diff : (p1 > p2) - (p1 < p2);

The second expression is the sign of the difference between the two pointers, but it’s guaranteed to return something good even if you subtract 64-bit pointers with 32-bit int, as on x86-64. In that case, with the current code, you could be unlucky and have two pointers that are more than 2G apart and get a wrong result.

> -- 
> 2.11.1
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel