2

If I had a loop that checks for a specific value in an array and, for some reason, must iterate over all elements and cannot break midway.

Which of the following would be more efficient: blindly setting a flag on each match, or checking if the flag is false before setting it.

bool happened = false;
while (...) {
  if (...) {
    happened = true;
  }
}

vs

bool happened = false;
while (...) {
  if (...) {
    if (!happened) happened = true;
  }
}

As far as I can tell, both are more or less equivalent on the assumption that memory reads are as fast as memory writes (ignoring the extra instruction in the second example). Am I correct in my conclusions?

Sam Parker
  • 31
  • 3
  • 2
    With the way your question is worded, it's hard to tell what you're asking exactly, but I'm thinking it will come down to your access patterns. Iterating over an array can be expensive, but so is branch misprediction. – AndyG Mar 10 '14 at 07:53
  • 4
    I doubt it would make a difference, except that the check looks somewhat silly. – Brave Sir Robin Mar 10 '14 at 07:54
  • While "efficiency" and any real performance difference can't be said without profiling, the first one seems better theoretically. Because when you decide to set `happened` to true, you don't have to worry about it's current value and extra check (in 2nd) seems a bit of overkill. – P.P Mar 10 '14 at 07:55

6 Answers6

4

The compiler will make the decision for you, if you use any meaningful optimization. Write whatever is cleanest and makes sense. To me that would be the first one, since it is less code and doesn't introduce more code paths. For fun, I did some tests in Clang 3.4 -O3:

bool happened = false;
extern volatile int dontOptMe1, dontOptMe2, dontOptMe3;
while (dontOptMe1) {
  if (dontOptMe2) {
    happened = true;
  }
}
dontOptMe3 = happened;

vs

bool happened = false;
extern volatile int dontOptMe1, dontOptMe2, dontOptMe3;
while (dontOptMe1) {
  if (dontOptMe2) {
    if(!happened) happened = true;
  }
}
dontOptMe3 = happened;

Resulted in the following in pseudo ASM:

  MOV      happened, 0
  BRA      LOOP_END
LOOP_START:
  SELECTEQ dontOptMe2, 0, happened, happened, 1
LOOP_END:
  BCZC     dontOptMe1, LOOP_START
EXIT:
  STORE    dontOptMe3, happened

vs

  MOV      happened, 0
  BCZS     dontOptMe1, EXIT
LOOP:
  SELECTNE dontOptMe2, 0, R2, 1, 0
  SELECTEQ happened, 0, R3, 1, 0
  AND      R3, R2, R3
  SELECTNE R3, 0, happened, 1, 0
  BCZC     dontOptMe1, LOOP
EXIT:
  STORE    dontOptMe3, happened

The first is much more desirable. This is also a good example of how restrictive volatile types are. I was surprised the compiler couldn't transform the second into the first.

Note: SELECTXX means, if Arg1 minus Arg2 sets condition code XX, set Arg3 to Arg4, otherwise set Arg3 to Arg5. So: SELECTNE dontOptMe2, 0, R2, 1, 0 is the equivalent of: R2 = (dontOptMe2 == 0) ? 1 : 0; in C

Sam Cristall
  • 4,328
  • 17
  • 29
3

Generally speaking, first version is more pipeline friendly, because its instruction stream is less disturbed by jump, therefore is more efficient. But it is depends on specific architecture features and compiler optimizations.

I believe the performance difference of these two versions is unnoticeable in actual situations.

Lee Duhem
  • 14,695
  • 3
  • 29
  • 47
1

Most answers so far seem to be "it depends", but I think it's pretty obvious:
If you conditionally set a value to something if it isn't that something already, it is logically identical to unconditionally setting the value. If you're lucky, the compiler will notice and treat both identically, but if it doesn't, the unconditional version wins every time.
1: It uses fewer instructions
2: The extra instructions are conditional, hurting branch prediction

If you're going with 

 if (cond) varx = vary;

The compiler uses one conditional branch (possibly a conditional move instead of a branch, if it is supported in the hardware)

If you're going with

 if (cond && varx != vary) varx = vary;

The compiler will either simplify to the first case, or use two conditional jumps (or one jump and a conditional move).

EOF
  • 6,273
  • 2
  • 26
  • 50
0

Only profiling will tell for sure, but most likely the variable is stored in the register anyway, not in memory (unless you have many other local variables in the loop) and there will be no measurable difference at all.

Suma
  • 33,181
  • 16
  • 123
  • 191
0

Yes, you are quite correct. They are more or less the same.

To be exact, the first form might be preferrable if the "happened" happens quite often, while the second one might want to be favourable if the "happend" is quite rare. But even then, I don't think the second one is better in any way.

In the end, it is probably a micro-optimization where it is better to go for readability instead of performance.

glglgl
  • 89,107
  • 13
  • 149
  • 217
0

I have run it empirically on the following code:

bool happened = false;
for (int i = 0; i < 1000000000; i++) {
  if (i % 2) {
    // uncomment the one you want to use
    // happened = true;
    // if (!happened) happened = true;
  }
}

I ran it 10 times on each, getting a mean of 2.304s with standard deviation 0.013s on the first and a mean of 2.399s with standard deviation 0.007s on the second. A two-sample t-test shows that happened = true; is faster than if (!happened) happened = true; and the difference is statistically significant with with p = 7e-12.

Tony
  • 246
  • 2
  • 8
  • The example seems simplified down too much. If you are profiling optimized code, any decent compiler will optimize the happened variable out (and most likely it will optimize out the whole loop). Profiling unoptimized code has no sense. – Suma Mar 10 '14 at 08:05
  • I compiled with `-O0`, and checked the assembly to make sure everything was there, so I can confirm that I am running on unoptimized code. I disagree with your sentiment that profiling unoptimized code makes no sense - the two binaries are the same except for what goes on in the innermost loop. So the only thing that changes is the thing we're comparing. – Tony Mar 10 '14 at 08:19