Sign in to follow this  
bencelot

massive if's vs continue

Recommended Posts

bencelot    204
Hey all, Just wondering what's faster between using huge if statements, vs using continues. Here's an example for healing all the living players on team 2 with less than 75 hp: //Using large if statements for(int i=0; i < player.size(); i++) { if(player[i].IsAlive() && player[i].GetTeam() == 2 && player[i].GetHP() < 75) player[i].Heal(); } //OR using continues for(int i=0; i < player.size(); i++) { if(!player[i].IsAlive()) continue; if(player[i].GetTeam() != 2) continue; if(player[i].GetHP() >= 75) continue; player.Heal(); } Are the continues faster, because they can stop checking once a criteria is not met? Or are they slower because they require more if calls or something.. If someone could explain to me (or point me to a useful article) about how this works, I'd greatly appreciate it. Thanks

Share this post


Link to post
Share on other sites
Driv3MeFar    1080
Quote:
Original post by bencelot
Are the continues faster, because they can stop checking once a criteria is not met?


The same happens with the statements in the first if block you posted. Logical operators like && can be short-circuited, such that when it becomes impossible for the whole statement to be true, the rest of it is not evaluated.

The two code blocks you posted will be near as makes no difference the same speed, so choose the one that is the easiest to read an makes the most sense. Even if they are slightly different, it's likely not worth optimizing because it will not be a bottleneck. In general, profile to determine your slowest code, then fix it at an algorithmic level, not with hackery like this.

Share this post


Link to post
Share on other sites
nb    172
some compilers / compiler settings actually do the if statements either way you suggested. They can be set so that they stop checking once a section of the if statement fails, or they can check the entire expression regardless of if any have already failed or not. Look into your settings and you'll find it i'm sure. You are trying to enforce a situation on the compiler it already caters for.

In Delphi it is referred to in the project options as 'complete boolean eval' which is unset, which is saying that it does NOT check each item in an 'if' statement if it already comes across a section that will fail the test.

It's most likely already set correctly. So just code whichever way is more productive for you. Use the if's if that serves you best, or the continues. Sometimes it can be more about layout and being readable than being 'fast', but even then you can span the if expression over multiple lines anyway.

Share this post


Link to post
Share on other sites
Crisium    100
Hi bencelot,

optimizing is an art.

as Driv3MeFar stated is correct.

but... if you want to gain performance sometimes restructing what you are doing is what you want to do. The best optimization is actually doing nothing at all.

I can see you have a value for HP and a flag for alive... why not combine them. Also having more then one list could really improve your performance no need having dead players in a list..move them out of the list and into another.

I can see that the for-next is handling healing and only applies them to players that should get healing... you could create a list that contains references to players that are valid for healing. Then you for-next would look like this:

for(int i=0; i < player.size(); i++) {
playerHealable.Heal();
}

This way you will only have to add/remove players from that list when something has changed that would cause them to be added or removed.

hope this helps,
Peter Wraae Marino


Share this post


Link to post
Share on other sites
Hodgman    51227
Quote:
Original post by Driv3MeFar
The same happens with the statements in the first if block you posted. Logical operators like && can be short-circuited, such that when it becomes impossible for the whole statement to be true, the rest of it is not evaluated.


^^^ Take this knowledge on board if you do want to optimise your if statements at some point.

Lets say for example that Test#1("player[i].IsAlive()" ) takes 5ms to execute, but Test#2("player[i].GetTeam() == 2") and Test#3("player[i].GetHP() < 75") only take 1ms. (I know these numbers are absurd, it's just for illustrative purposes!)

To make your loop as fast as possible, you should order these tests within the if from fastest to slowest. i.e.:
if( player[i].GetTeam() == 2 && // 1ms
player[i].GetHP() < 75 && // 1ms
player[i].IsAlive() ) // 5ms


This way, if either of the fast tests return false, then the slow test will never be executed. I.e. it only calls the 5ms function on objects which have passes the quick tests.

For a real-world example, physics engines often use bounding-spheres to check for possible collisions *before* doing a real collision test. This is because a sphere-test is really quick, but a real collision test is really slow.

Share this post


Link to post
Share on other sites
Quote:
Original post by nb
some compilers / compiler settings actually do the if statements either way you suggested. They can be set so that they stop checking once a section of the if statement fails, or they can check the entire expression regardless of if any have already failed or not.
Careful, though. This may be a "speciality" of Delphi (don't know anything about Delphi really).
In C/C++, it is clearly against the standard (you are guaranteed that the right side of && will not execute if the left side is false), and any compiler implementing it differently is broken. I wouldn't want to use a compiler that lets you customize such a behaviour, since many programmers indeed rely on that assertion, so it may break code that's perfectly valid.

Share this post


Link to post
Share on other sites
SnotBob    202
The if statement is definitely better. As others have said, this is not a performance issue at all, but one of coding style. The reason why the if statement is better is that it's easier to refactor. Let's say that you need to do something else besides healing. You can just put another statement into the loop, but with continues like that, you'd need another loop. Also, it's somewhat less typing.

If you think that the statement is too wide, you can always do this:

if ( player[i].IsAlive()
&& player[i].GetTeam() == 2
&& player[i].GetHP() < 75)
player[i].Heal();

Share this post


Link to post
Share on other sites
nb    172
Quote:
Original post by samoth
Quote:
Original post by nb
some compilers / compiler settings actually do the if statements either way you suggested. They can be set so that they stop checking once a section of the if statement fails, or they can check the entire expression regardless of if any have already failed or not.
Careful, though. This may be a "speciality" of Delphi (don't know anything about Delphi really).
In C/C++, it is clearly against the standard (you are guaranteed that the right side of && will not execute if the left side is false), and any compiler implementing it differently is broken. I wouldn't want to use a compiler that lets you customize such a behaviour, since many programmers indeed rely on that assertion, so it may break code that's perfectly valid.


i did say 'some'. as a default delphi does not check all statements if one is already found to be crap. i'm sure there's a reason why it can be toggled to do checks on all statements... maybe something to do with being able to debug values that it would otherwise trash and render un-readable when it got to the if statement. don't know. all's good.

Share this post


Link to post
Share on other sites
Assuming that your GetXXX() IsXXX() functions are typical inline const accessors, which they probably are, you might even consider having them all evaluated and using & instead of &&. But stop! Read to the end before thinking about that.

Since & is not the same thing as &&, a lot of care has to be taken. In the above example, IsAlive() must return bool, else using & will break your program. Generally, you should never think about such optimisations unless it's really necessary. They make your code more error-prone, which isn't good. Still, this is FYI.

The reasoning behind using & is that if the getter functions are mere accessors, the branch instructions generated by either && or continue will by far be the most expensive thing in this situation, especially if a branch is mispredicted.
& will generate an "and" instruction which is fast and can't mispredict. However, & should only ever be used if everything is bool and has no side effects (and always with great caution), since otherwise you may have very hard to track bugs.
For example, if IsAlive() just returns the hp value of the player in an int, then if(IsAlive()) will work just fine, but if(GetGroup() == 5 & IsAlive()) will not work correctly. The worst thing is, it will actually work in 50%, which makes it such a pain to track down...

Share this post


Link to post
Share on other sites
Rydinare    487
I agree with the sentiment of using the if statement, rather than multiple if statements with continues. I can safely say that in my many years of programming, I've never found a case where I needed to use continue. I also would say that the majority of developers I know don't use continue often. So, there's certainly the factor that code with continue might take a little longer to get the grasp of for someone else.

Having said all that, on a slight tangent, if it's common case that you're checking for the same few conditions in multiple areas, I would advise making a utility function that checks all three, to keep the client code a bit cleaner.

Something like:


bool needsToBeHealed(const Player& p_player, TeamId p_teamId, HitPoints p_minHitPointThreshhold)
{
return p_player.IsAlive() && p_player.GetTeam() == p_teamId && player[i].GetHP() < p_minHitPointThreshhold;
}

// client code
if (needsToBeHealed(player[i], 2, 75))
{
player[i].Heal();
}



I do think that may help readability. If the condition is only tested in one place in the code, then this may be overkill.

Share this post


Link to post
Share on other sites
sphen_lee    199
Quote:
Original post by nb
Quote:
Original post by samoth
Quote:
Original post by nb
some compilers / compiler settings actually do the if statements either way you suggested. They can be set so that they stop checking once a section of the if statement fails, or they can check the entire expression regardless of if any have already failed or not.
Careful, though. This may be a "speciality" of Delphi (don't know anything about Delphi really).
In C/C++, it is clearly against the standard (you are guaranteed that the right side of && will not execute if the left side is false), and any compiler implementing it differently is broken. I wouldn't want to use a compiler that lets you customize such a behaviour, since many programmers indeed rely on that assertion, so it may break code that's perfectly valid.


i did say 'some'. as a default delphi does not check all statements if one is already found to be crap. i'm sure there's a reason why it can be toggled to do checks on all statements... maybe something to do with being able to debug values that it would otherwise trash and render un-readable when it got to the if statement. don't know. all's good.


As samoth says, any C/C++ compiler that doesn't short-circuit logic is very broken. It breaks a very common if statement involving NULL pointers:
if(ptr && *ptr == some_value)...
If the compiler doesn't short-circuit the &&, it will dereference a null pointer.

Share this post


Link to post
Share on other sites
godecho    235
I wouldn't worry about optimizations like this, at this level. The important thing is that the code works correctly, is readable, and maintainable. The time to deal with these kinds of micro optimizations is when a profiler suggests that it's a bottleneck. If this is for a game, the cost of several if's is negligible compared to the graphics operations.

Premature optimization *rarely* works to your advantage.

Share this post


Link to post
Share on other sites
rozz666    896
Quote:
Original post by Hodgman
Quote:
Original post by Driv3MeFar
The same happens with the statements in the first if block you posted. Logical operators like && can be short-circuited, such that when it becomes impossible for the whole statement to be true, the rest of it is not evaluated.


^^^ Take this knowledge on board if you do want to optimise your if statements at some point.

Lets say for example that Test#1([i]"player.IsAlive()" ) takes 5ms to execute, but Test#2([i]"player.GetTeam() == 2") and Test#3([i]"player.GetHP() < 75") only take 1ms. (I know these numbers are absurd, it's just for illustrative purposes!)

To make your loop as fast as possible, you should order these tests within the if from fastest to slowest. i.e.:
if( player[i].GetTeam() == 2 && // 1ms
player[i].GetHP() < 75 && // 1ms
player[i].IsAlive() ) // 5ms


This way, if either of the fast tests return false, then the slow test will never be executed. I.e. it only calls the 5ms function on objects which have passes the quick tests.


Actually, you could do better. You could try to predict the probabilities of each test's failure. Assume ti is the time of the ith condition and pi is it's probability of failure. The first takes average time of t1. The second (1 - p1) * t2. The third (1 - p1) * (1 - p2) * t3. This is true under the assumption that p1, p2 and p3 are uncorrelated. So the total average time is: t1 + (1 - p1) * t2 + (1 - p1) * (1 - p2) * t3. So you have to put your conditions in such order that the total time is minimized.

Share this post


Link to post
Share on other sites
Wan    1366
Quote:
Original post by Rydinare
Having said all that, on a slight tangent, if it's common case that you're checking for the same few conditions in multiple areas, I would advise making a utility function that checks all three, to keep the client code a bit cleaner.

Something like:

*** Source Snippet Removed ***

I do think that may help readability. If the condition is only tested in one place in the code, then this may be overkill.

I agree. One might even consider to keep the check inside the class itself. YMMV of course.


void Player::heal()
{
strength = 100;
}

void Player::update()
{
if (shouldHeal())
{
heal();
raisePlayerHealedEvent(this);
/* or something... */
}
}

bool Player::shouldHeal()
{
return (IsAlive() && ..);
}




Share this post


Link to post
Share on other sites
Antheus    2409
3 ifs are not *massive*. 250kb source code of thousands of ifs is massive.

If you're using OO, one alternative is this:

for (Healer h : healers) {
player->HealPartyMembers();
}

...

bool Player::NeedsHealing()
{
return alive && health < 75;
}

void Healer::HealPartyMembers()
{
for (Player p : my_team) {
if (p->needsHealing()) Heal(p);
}
};



The team is implicit property of a player, NeedsHealing accesses local variables, Healer (also player) may be a function limited to Healer <- Player if using OO, whereas NeedsHealing is common to all players.

The above can be done in OO, functional or procedural manner.

Share this post


Link to post
Share on other sites
King of Men    394
As has been noted, this is no good as an optimisation. If nothing else, the compiler is likely to implement the two possibilities in the same way anyway. As a point of coding style, though, I prefer the continues as being easier to read, because the lines are shorter and there is one logical statement per line. No need to parse logical operators or parens, in cases where that's significant. This is a question of taste, though. Continues are also nice for avoiding nesting that pushes your whole code to the right.

Share this post


Link to post
Share on other sites
SimonForsman    7642
Quote:
Original post by Hodgman
Quote:
Original post by Driv3MeFar
The same happens with the statements in the first if block you posted. Logical operators like && can be short-circuited, such that when it becomes impossible for the whole statement to be true, the rest of it is not evaluated.


^^^ Take this knowledge on board if you do want to optimise your if statements at some point.

Lets say for example that Test#1([i]"player.IsAlive()" ) takes 5ms to execute, but Test#2([i]"player.GetTeam() == 2") and Test#3([i]"player.GetHP() < 75") only take 1ms. (I know these numbers are absurd, it's just for illustrative purposes!)

To make your loop as fast as possible, you should order these tests within the if from fastest to slowest. i.e.:
if( player[i].GetTeam() == 2 && // 1ms
player[i].GetHP() < 75 && // 1ms
player[i].IsAlive() ) // 5ms


This way, if either of the fast tests return false, then the slow test will never be executed. I.e. it only calls the 5ms function on objects which have passes the quick tests.

For a real-world example, physics engines often use bounding-spheres to check for possible collisions *before* doing a real collision test. This is because a sphere-test is really quick, but a real collision test is really slow.


Actually its not always the best idea to sort the checks in the order fastest to slowest, you also should consider the fail/success rates, a check that fails 99% of the time is a better choice for the first check than one that is 50% faster but only fails 20% of the time.

Share this post


Link to post
Share on other sites
Rydinare    487
Quote:
Original post by WanMaster
Quote:
Original post by Rydinare
Having said all that, on a slight tangent, if it's common case that you're checking for the same few conditions in multiple areas, I would advise making a utility function that checks all three, to keep the client code a bit cleaner.

Something like:

*** Source Snippet Removed ***

I do think that may help readability. If the condition is only tested in one place in the code, then this may be overkill.

I agree. One might even consider to keep the check inside the class itself. YMMV of course.

*** Source Snippet Removed ***


I thought about putting it in the Player class when I first mentioned it as an example. My main reason for not suggesting such is the open-closed principle. Sounds like all of the internals could be accomplished through the public interface of the Player class, and therefore the Player class doesn't really to take the hit of having utility sorts of modifications. Of course, this is at the risk of promoting feature envy; an interesting trade-off.

Share this post


Link to post
Share on other sites
Hodgman    51227
Quote:
Original post by SimonForsman
Actually its not always the best idea to sort the checks in the order fastest to slowest, you also should consider the fail/success rates, a check that fails 99% of the time is a better choice for the first check than one that is 50% faster but only fails 20% of the time.

Yeah, rozz666 already pointed that out ;)

Quote:
Original post by frob
It looks like you are writing a letter!

Sorry, couldn't help it ^^

Share this post


Link to post
Share on other sites
silvermace    634
looks an awful lot like premature optimisation to me. I second the person that said make the code correct, readable and maintainable.

IMO I doubt any syntatic change you make will matter so long as the predicates are the same and the overall result of the expressions are the same, the compiler will almost certainly reorganise your statements to maximize things you arn't even worrying about yet (cache, BPU hit rate, instr. pipeline/mem stalls etc)

On a side note, and just out of interest.
This is an excellent candidate for paralleling.

PS> this is off the top of my head, probably won't compile

typedef std::vector< Player >::iterator player_iter;
void heal_thread_func(player_iter begin, player_iter end)
{
for( player_iter i = begin; i != end; ++i ) {
if( i->GetTeam() == 2 && i->GetHP() < 75 && i->IsAlive() ) {
i->Heal();
}
}
}

void heal() {
boost::thread_group* tg = new boost::thread_group();
const int c = player.size() / 3;
tg->create_thread( boost::bind(&heal_thread_func, player.begin(), player.begin() + 1*c );
tg->create_thread( boost::bind(&heal_thread_func, player.begin() + 1*c, player.begin() + 2*c );
tg->create_thread( boost::bind(&heal_thread_func, player.begin() + 2*c, player.end() );
tg->join_all();
}


Share this post


Link to post
Share on other sites
iMalc    2466
I would be VERY surprised if your compiler didn't produce identical assembly code for both ways of doing it.
It is really only a matter of taste. Use whichever is clearer.

Share this post


Link to post
Share on other sites
nb    172
Quote:
Original post by sphen_lee
Quote:
Original post by nb
Quote:
Original post by samoth
Quote:
Original post by nb
some compilers / compiler settings actually do the if statements either way you suggested. They can be set so that they stop checking once a section of the if statement fails, or they can check the entire expression regardless of if any have already failed or not.
Careful, though. This may be a "speciality" of Delphi (don't know anything about Delphi really).
In C/C++, it is clearly against the standard (you are guaranteed that the right side of && will not execute if the left side is false), and any compiler implementing it differently is broken. I wouldn't want to use a compiler that lets you customize such a behaviour, since many programmers indeed rely on that assertion, so it may break code that's perfectly valid.


i did say 'some'. as a default delphi does not check all statements if one is already found to be crap. i'm sure there's a reason why it can be toggled to do checks on all statements... maybe something to do with being able to debug values that it would otherwise trash and render un-readable when it got to the if statement. don't know. all's good.


As samoth says, any C/C++ compiler that doesn't short-circuit logic is very broken. It breaks a very common if statement involving NULL pointers:
if(ptr && *ptr == some_value)...
If the compiler doesn't short-circuit the &&, it will dereference a null pointer.


exactly the same thing for delphi... my last point was more that i do not know the reasoning behind NOT short-circuiting, and can only assume it is for what i already mentioned.

Personally i find that it's sometimes cleaner to put the if over multiple lines if as previously discussed the statement pushes too far to the right


if (a = b) and
(c = d) and
(so_on = and_so_on) then begin
do_something_cool;
end;



i would also add that it would be wise to put a comment near the first part of the if that says something like "make it easier to read" so that you don't suddenly come across a piece of your code formatted different and have to wonder why you did that and why is it appears to be special etc.

Share this post


Link to post
Share on other sites
Steadtler    220
To add my grain of salt, always profile before optimizing. Else you're just wasting your time.

But always use the pre-operator (++i) unless you have a good reason not too...

Also, if that code is real, you have bigger performance problem than && vs continue. For example, you are calling your [] operator 4 times when one would suffice, and thats inside the loop, and the call to .size() should be taken outside the loop...

IMO I would have written it like this:


const unsigned int maxPlayer = playerarray.size();
for(int playerIndex = 0; playerIndex < maxPlayer ; ++playerIndex )
{
PlayerType& player = playerarray[playerIndex ];
if(player.GetTeam() == 2 && player.GetHP() < 75 && player.IsAlive())
{
playerHeal();
}
}



Now thats a start. If you want to make it clean, it would look something like this:


for_each(playerarray.begin(), playerarray.end(), ConsiderHealing());



or something.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this