views:

623

answers:

12

I know- Premature optimization.
But I've got code that's supposed to find out if a position is changed vs a cached position.

Current code is:

if(abs(newpos-oldpos) > 1){
    .....
}

Is it more efficient to use the following?

if(abs(newpos-oldpos) != 0){
    ....
}

Why or why not? I'm currently debating it my head which is more readable and was wondering if there was a performance difference I was missing.

+13  A: 

Why not this?

if (newpos != oldpos) {
    ...
}

More efficient than both due to the lack of abs(), and clearer to boot.

Noah Medling
(newpos != oldpos) is not the same as (abs(newpos-oldpos) > 1).
Michael Burr
But it is the same as (abs(newpos-oldpos)!=0). The question was about equality, and either one of the pieces of code is wrong (my bet is the first) or Luciano has bigger problems than the efficiency of comparison operators.
Noah Medling
I agree that (newpos != oldpos) is superior in every way than (abs( newpos-oldpos) != 0). However, bringing (abs(newpos-oldpos) >1) into the mix I think changes what the answer should be (especially as it's the original code that you would presumably want to preserve the behavior of).
Michael Burr
+2  A: 

Don't try to optimize comparison operators to equal operators; they should have the same timing, and only have the same value for integers.

If you're going to optimize at all, try

if ((newpos - oldpos > 1) || (oldpos - newpos > 1))

which is still readable. (as well as consistenly correct for floating pt #s)

edit: ack! Never mind, if you want to know whether position has changed by some minimum delta (I originally read your code question literally w/o seeing the overall goal you're trying to achieve), use this:

if ((newpos - oldpos > delta) || (oldpos - newpos > delta))

for delta > 0, or this (as Noah M suggested)

if (newpos != oldpos)

for delta = 0

Jason S
Avoiding the overhead of abs may well be a much faster solution.
Steven Sudit
A: 

The performance difference would be miniscule, but the first one would be more efficient (from my guess) b/c it involves less operations than !=. Also, the 2 statements mean different things, for example, try abs(newpos - oldpos) = 0.5 and see, unless the two variables are integers.

stanigator
I don't think so: I don't think it involves less operations than !=.
ChrisW
On x86, comparison sets flags that indicate zero, greater than, etc. - so they'd be the same cost wise
Michael
+3  A: 

On most architectures, there should be no difference. Comparisons are typically done inside a CPU by doing a subtraction and letting the condition codes get set by the ALU. Branching is done by testing condition codes (ie, branch on not equal tests the zero bit in the condition code register, branch on greater typically tests the zero, negative, and overflow flags).

plinth
+4  A: 

The second is more efficient if you remove the unnecessary abs(). If you're comparing against zero, it doesn't matter whether the difference is positive or negative. Also, I think it's more readable. However, the two conditionals don't seem equivalent; what happens if abs(newpos-oldpos) == 1?

Nick Lewis
+13  A: 

A major reason not to change

(abs(newpos-oldpos) > 1)

to

(abs(newpos-oldpos) != 0)

is that they are semantically different.

When abs(newpos-oldpos) == 1 you get different results. This is an example of why you should be reluctant to change things 'just because' - aside from the fact that there would be no measurable difference in performance anyway (and probably no actual difference either).

Michael Burr
Yes, I see that now. Semantics are already changing for this Proof-of-Concept (change in unit), so I really do want it to change it to delta of 0....I'm going to have to invest more time in this code change than I originally planned, as thinking about it more I may be able to completely remove this conditional now.
Luciano
+1  A: 

Ignoring the fact they are not equivilent operations, on x86, you might be able to save a cycle or so.

abs(new-pos) > 1

Would be

  1. Subtraction
  2. Abs
  3. Compare to 1
  4. Jmp

abs(newpos-oldpos) != 0

Would be

  1. Subtraction
  2. Abs
  3. Jmp - If abs is inlined and the last operation appropriately sets the zero flag.

I'd be surprised if this has any measurable impact on your program - You definitely deserve praise if your code is already running that incredibly tight.

Michael
+3  A: 

Unless I am missing something, they do not do the same thing. x > 1 is not the same as x != 0.

anon
abs is never less than zero. Therefore, abs(x) != 0 implies abs(x) >= 1. Using >, however, is a bug :)
bdonlan
Thanks for explaining the behaviour of abs() to me, As it happens, I already knew that.
anon
+1  A: 

Instead of guessing what the compiler will do, why not just look at the resulting assembly code, or measure its execution?

Neil
A: 

I don't now if it's me who is missing something, but that code is on every book about floating point numbers and it is intended to get a positive match between two slightly different numbers.

If the code has to compare two floats there is no possible optimization on most machines but the important thing is that this is not about premature optimization, it is about refactoring code you don't fully understand.

901
A: 

Since the answer will depend on your architecture, let's take a look at the generated code on x86-64 (with gcc -O3):

#include <math.h>

int t_gt(int x) { // note! not equivalent to the others
    return abs(x) > 1;
}

int t_ge(int x) {
    return abs(x) >= 1;
}

int t_ne(int x) {
    return abs(x) != 1;
}

becomes:

Disassembly of section .text:

0000000000000000 <t_gt>:
#include <math.h>

int t_gt(int x) {
   0:   89 f8                 mov    %edi,%eax
   2:   c1 f8 1f              sar    $0x1f,%eax
   5:   31 c7                 xor    %eax,%edi
   7:   29 c7                 sub    %eax,%edi
   9:   31 c0                 xor    %eax,%eax
   b:   83 ff 01              cmp    $0x1,%edi
   e:   0f 9f c0              setg   %al
    return abs(x) > 1;
}
  11:   c3                    retq   
  12:   66 66 66 66 66 2e 0f  nopw   %cs:0x0(%rax,%rax,1)
  19:   1f 84 00 00 00 00 00 

0000000000000020 <t_ge>:

int t_ge(int x) {
  20:   89 f8                 mov    %edi,%eax
  22:   c1 f8 1f              sar    $0x1f,%eax
  25:   31 c7                 xor    %eax,%edi
  27:   29 c7                 sub    %eax,%edi
  29:   31 c0                 xor    %eax,%eax
  2b:   85 ff                 test   %edi,%edi
  2d:   0f 9f c0              setg   %al
    return abs(x) >= 1;
}
  30:   c3                    retq   
  31:   66 66 66 66 66 66 2e  nopw   %cs:0x0(%rax,%rax,1)
  38:   0f 1f 84 00 00 00 00 
  3f:   00 

0000000000000040 <t_ne>:

int t_ne(int x) {
  40:   89 f8                 mov    %edi,%eax
  42:   c1 f8 1f              sar    $0x1f,%eax
  45:   31 c7                 xor    %eax,%edi
  47:   29 c7                 sub    %eax,%edi
  49:   31 c0                 xor    %eax,%eax
  4b:   83 ff 01              cmp    $0x1,%edi
  4e:   0f 95 c0              setne  %al
    return abs(x) != 1;
}
  51:   c3                    retq

As you can see, there are two differences:

  • The condition codes on the set* ops are different. This likely won't affect speed.
  • For t_ge, a two-byte register test (AND) is used, while the other two use cmp (subtraction) and a literal one-byte operand to compare against.

While the difference between the various set* variants, and between test and cmp is likely zero, the additional one-byte operand to cmp may decrease performance by an immeasurably small amount.

Of course, the best performance will be given by getting rid of the pointless abs() entirely:

0000000000000060 <t_ne2>:

int t_ne2(int x) {
  60:   31 c0                 xor    %eax,%eax
  62:   85 ff                 test   %edi,%edi
  64:   0f 95 c0              setne  %al
    return (x != 0);
}
  67:   c3                    retq

Keep in mind that these findings may not apply on other architectures; however losing the abs is sure to be faster anywhere.

bdonlan
A: 

At least on the compiler that I'm using at the moment (gcc 4.2), the assembly code that it generates for your first expression employs the trick described here. Then it decrements the result and uses the condition code to decide how to branch.

For your second, it rewrites it to something that's basically, newpos != oldpos

The two expressions that you gave are slightly different in meaning. But either way, most reasonable compilers that I've seen deploy some very interesting tricks to micro-optimize your code. You'd be hard pressed to outdo the compiler in this regard. Your best bet is the usual advice: try both versions, profile the code, and see which actually executes more quickly.

By the way, if you'd meant abs(newpos - oldpos) >= 1 for the first test, it still generates the absolute value sequence. That's probably because of the possibility of overflow in the subtraction, assuming a two's complement system. On my machine, for example, abs(-2147483648 - 2147483647) gives 1 which would fail your test if you were looking for a delta of say, 2 or more instead even though they're clearly quite different. The compiler's optimizer has to be careful to preserve this kind of behavior even on edge cases.

Boojum