Sign in to follow this  
Promit

Removing assertions from release builds -- a bad idea?

Recommended Posts

Promit    13246
First of all, a bit on what assertions are for. Consider the following quotes from Alexandrescu:
Quote:
One important thing about assertions is that they generate code in debug mode only (when the NDEBUG macro is not defined), so they are in a sense "free." This allows you to test for truths that seem so obvious that it would be a waste of cycles to verify them. But cycles are not wasted in release mode, so you have the psychological comfort to insert assertions liberally. ... There is an effective litmus test to differentiate the cases in which you need to use assert and when you need to use genuine error checking: you use error checking for things that could happen, even very improbably. You use assert only for things that you truly believe cannot possibly happen under any circumstances. An assertion that fails always signals a design or a programmer error — not a user error.
So, in a perfectly correct program (ha!), an assert will never trigger, debug or release. Now, it seems to me that there's absolutely no reason to disable asserts in your release build. The one and only time it would make sense to do so is if you're using asserts in tight performance critical code, and in that case it might be useful to have a seperate macro that is removed from the build. That aside, though, why remove the asserts? Most asserts are little more than a compare and a branch if the assert fails (and the branch should be predicted correctly after a hit or two). An assert is a condition that you believe can never happen. If it does happen, it doesn't matter whether it's a debug or release build. The only difference is that in the release build, you might not find out until it's much too late -- if you find out at all. So, why should we bother dropping the asserts at all? The performance impact is laughable unless your assert frequency is really very dense (like, an assert for every 5-10 lines of code or something). They check important conditions that in the best case will never trigger anyway, and in reality will probably trigger every so often. Consider that Solaris and Windows both ship with assertions enabled in the kernel. Several Windows BSODs, such as the "Driver IRQL NOT LESS THAN OR EQUAL TO" BSOD, are actually assertions in the kernel triggering. Things that were never supposed to be possible, except that they went and happened anyway. I believe UT 2k4 also has assertions enabled when it ships, but I'm not quite sure (a few friends say they've seen it assert on occasion). Removing asserts just seems like a good way to mask bugs, particularly those that might only appear in release mode.

Share this post


Link to post
Share on other sites
Agony    3452
In code that I've seen, asserts check to make sure that the current state/data is valid, but doesn't do anything if it isn't. Error checking code, on the other hand, does have a course of action to take if things are bad.

So with an assert, if you left it in for release mode, it would simply crash the program anyway. But the user would possibly get a message box saying so, using language that would only mean something useful to the developer. Might as well let it crash (since it's going to anyway), rather than cost a wee little bit of processing time to gain absolutely no worthwhile functionality.

If you've added release-mode functionality (such as gracefully handling an error), then it's no longer an assert, it's healthy error checking.

Share this post


Link to post
Share on other sites
Guest Anonymous Poster   
Guest Anonymous Poster
One reason to remove asserts in release code is that they can be replaced with something telling the compiler 'this will not happen' which can enable more optimization. The command in VS to do this is __assume ( http://search.microsoft.com/search/results.aspx?qu=__assume&View=msdn&st=b&c=0&s=1&swc=0 )
The idea is that in debug, assert() will report the problem and exit, while in release assert() turns into __assume() so the compiler makes assumptions about what is and isn't possible.

It sounds like you're talking about an error logging system, which is something completely different. Asserts are for testing reqirements(comparable to preconditions and postconditions) basically - if a function has a defined effect when you pass a null pointer, that is a place error checking. If it is a requirement that you never give it a null pointer, that is a place for an assert().

-Extrarius

Share this post


Link to post
Share on other sites
RobTheBloke    2553
haha, i actually assert almost every line of code i write - in addition i provide extra debug utils to trace through my code execution, check every memory read/write to ensure valid, log every argument passed into a function and various other debug tests.

The point is, that if (or rather when in my case) all asserts have passed OK, i'm pretty certain that the code is all good. There is a difference is Knowing that your code is bug free, and thinking that it is bug free.

The checks that get left in release build are simply for input into the system - ie, data from files, executed lua scripts, user input etc.

There are reasons you want to get rid of un-nessacary asserts, and that is primarily silly amounts of text strings. ie, the assert macro is pretty much :


#define assert(X) if(!(X)) { fprintf(stderr,"assert: %s : line=%d : file=%s\n",#X,__LINE__,__FILE__); exit(0); }


The #X will be replaced with your check (as text string) and the __FILE__ will probably get replaed with "C:/Documents & Settings/rob/My Documents/Project/Src/Main.cpp". An awful lot of text data there ! In my current little plugin i'm writing, i loose just under 500Kb in compiled size when removing asserts alone!

Basically, if you have a system such as

A -> Outputs to -> B -> Outputs to -> C -> Outputs to -> D

If you KNOW that the code that takes you from A to D is 100% correct (ie, if given valid data, it will always work), then you only really need to check that the data entering at pointA is correct -> all other error checking is surplus to requirement.

Share this post


Link to post
Share on other sites
Koshmaar    989
I'm using SC_ASSERT macro in some very speed important functions, ie. RenderText - IMHO it's obvious that in such situations tests should be ommited in release builds. However, there are many places in my code, that are subject to user actions, and sometimes, under very rare circumstances, they may produce bugs.

But they're so low level and they appear in so many places, that it would be really annoying to write specialized code for each one, so I developed special assert version - SC_VERIFY - which isn't removed from release build. This way, when sth bad happens, program will immediately crash - but IMHO it's acceptable in those rare cases, and IMHO - it's much better than leaving user with application with undefined behaviour (should we call it wild application, just like wild pointer? [wink])

I know that's called bad programming habit... but I don't want to spend > 30% of my time writing code which will greacefully handle errors... especially in an engine which's eventually's going to be replaced with new one.

Share this post


Link to post
Share on other sites
Promit    13246
Quote:
Original post by ace_lovegrove
asserts are compiled out in Release builds anyways.


And I'm saying they shouldn't be.

Quote:

It sounds like you're talking about an error logging system, which is something completely different.

I never mentioned logging. An assert is an unrecoverable condition, and calling more code could exacerbate the problem. Imagine if you tried to run the assert data through your logging class, which itself has asserts. If one of those triggers, you're re-entrant and potentially in serious trouble. The only thing you can do is to abort(). Asserts are not error handling. Asserts are not graceful. Leaving asserts enabled in a release build is not a method of keeping your app from going tail up. What they are is a way of catching the mistake at the point of occurrence and making a note of that point, so that when your user complains, you know exactly where the error happened.

Quote:

So with an assert, if you left it in for release mode, it would simply crash the program anyway.

In the case that an assert condition does fire, we can assume that something has gone seriously wrong. Wrong enough that it is almost given that even if we don't crash immediately, we're going to do so very soon. So given that we are going to crash no matter what, we have two options. Either we can show the user something like this friendly little dialog:

Or we can output recoverable debug information (in this case, condition/line/file) and abort(). Notice that this involves writing your own assert, that is a little more clear than the normal library assert. Does the message mean anything to the user? Of course not. But if they can email you the output of the failure, you have some starting place for fixing the bug. If you get the "Complain to MS" dialog, how are you supposed to debug it? Where would you even start?

Share this post


Link to post
Share on other sites
Promit    13246
Quote:
Original post by Koshmaar
I'm using SC_ASSERT macro in some very speed important functions, ie. RenderText - IMHO it's obvious that in such situations tests should be ommited in release builds. However, there are many places in my code, that are subject to user actions, and sometimes, under very rare circumstances, they may produce bugs.

But they're so low level and they appear in so many places, that it would be really annoying to write specialized code for each one, so I developed special assert version - SC_VERIFY - which isn't removed from release build. This way, when sth bad happens, program will immediately crash - but IMHO it's acceptable in those rare cases, and IMHO - it's much better than leaving user with application with undefined behaviour (should we call it wild application, just like wild pointer? [wink])


I do concede that there are performance critical sections of code that can benefit from removal of asserts, which is why I think using two macros is a good idea. I have the normal assert, which isn't removed, and a "fast assert", which is removed from the release build. I'd just use SC_VERIFY a lot more than SC_ASSERT.

Quote:

One reason to remove asserts in release code is that they can be replaced with something telling the compiler 'this will not happen' which can enable more optimization. The command in VS to do this is __assume ( http://search.microsoft.com/search/results.aspx?qu=__assume&View=msdn&st=b&c=0&s=1&swc=0 )
The idea is that in debug, assert() will report the problem and exit, while in release assert() turns into __assume() so the compiler makes assumptions about what is and isn't possible.

That's interesting, to say the least. But it strikes me as highly dangerous to do. Now, not only have you removed the assertion, but you've potentially changed the behavior of the code and the way the optimizer looks at the code. Maybe you'll gain a few cycles, but there's a risk of making debugging next to impossible, because any bugs that arise (particularly bugs in the compiler) will be pretty much impossible to ever track down and identify.

Share this post


Link to post
Share on other sites
Oxyacetylene    426
When I worked as a game tester, the builds we used for testing were release builds with the asserts still in.

I'm pretty sure the asserts were removed in the final build though.

Share this post


Link to post
Share on other sites
sit    170
Quote:
Original post by Promit
Quote:
Original post by ace_lovegrove
asserts are compiled out in Release builds anyways.


And I'm saying they shouldn't be.

And I say that demonstrates you're using asserts wrong

... now, to find a good way to explain this ...

in my mind assertions are one step above a compilier error in your debug process. They should be used only to verify things that a programmer may have done wrong, not something the artist [or worse, user] did to the game assets or anything of the sort.

Coming up with a concise and relavant example isn't trivial.

say you have a min and max angle for something and they have to be in (-180,180] [degrees]. It is easy enough to check and complain if they are within that range when you set them. Lets say you aren't doing anything more fancy, and when you use the angles you add an assert that min is less than max. If these values were hard coded then the programmer should easily know what to do in response.

[the proper fancy thing to do would be to make setting the high angle to something lower than low also set low to the new high angle and vice versa]

as I can't find a good example of where asserts are all that useful in the first place [not that I belive they aren't useful, I just am at a loss of ideas] can you show one valid place where an assert in a release application [different from release build] would be better than exceptions or even checking the return value from a function?

Share this post


Link to post
Share on other sites
Nitage    1107
Asserts serve no purpose in a final product.

Asserts are supposed to be used to catch programmer errors.

Presumably, after an assert fails, you fix the error causing it (if it can't be fixed because it has an external trigger, then it need replaceing with an exception).

There's no point in releasing an exectuable containing code to catch errors you've already fixed (any bugs in your program won't have corresponding assertion failures, else you'd know about the bug and have fixed it).


Share this post


Link to post
Share on other sites
Promit    13246
Quote:
Original post by Nitage
There's no point in releasing an exectuable containing code to catch errors you've already fixed (any bugs in your program won't have corresponding assertion failures, else you'd know about the bug and have fixed it).


Are you ready to make the claim that your release code can never fail an assert condition, no matter what corner cases are reached?

Share this post


Link to post
Share on other sites
DaBono    1496
I guess the amount of use asserts have in release build depends on your sense of 'customer support'.

If you remove asserts and your game crashes, at least Win XP already shows the nice user friendly equivalent of 'You are screwed..'. In some cases, the game may even stumble along, possibly allowing the user to save his game just before going down. In short, the user is screwed anyway, is letting him down easy really helpful?

However, if you intend to fix your bus and release patches (as more and more games do) then the crash information is probably vital for a quick bug fix.

Share this post


Link to post
Share on other sites
Nitage    1107
Quote:
Original post by DaBono
However, if you intend to fix your bus and release patches (as more and more games do) then the crash information is probably vital for a quick bug fix.


That's what exceptions are for.

Share this post


Link to post
Share on other sites
Sphet    631
In my mind, asserting on cases is there to help with the debugging process. If you are shipping a release build of the game, and want to catch really bad cases, use exceptions. In a lot of applications, there are good mixtures:



bool PerformSomeTaskWithAUserFile( const char* szFilename )
{
// Everything before this should guarantee that the file
// pointed to is a valid file, but double check.
// None the less, if the file IS invalid, don't proceed.
// Only raise the warning so a tester can get a coder
// to figure out why this happened.

if ( IsFileValid( szFilename ) == false )
{
ASSERT_MSG( false, "File is invalid, this should not happen" );
return false;
}

// .. go on and do things with the file

return true;
}


I often put assertions in to let the testers know that something unnexpected has happened. When they are testing the program they know to get a coder or log the bug. In release at least the case is handled somewhat. The function returns false and the caller knows something bad has happened and can report this case to the user, or also handle the case gracefully.

I'm not saying that I do this in all cases, but I certainly try to make applications fail with some sense of grace in release. The end user is much happier knowing that something was wrong with the file instead of getting an assertion that the file isn't valid and then crashing.

Just my two cents.

- S

Share this post


Link to post
Share on other sites
SiCrane    11839
The thing about the standard assertion handler is that it basically crashes the program while producing an error message. If you mean to leave assertions in the release build, you need to change this default behaviour somehow to do things like more gracefully clean up resources, especially things like file handles. In this case, a proper exception handling discipline would probably handle this cleanup. So if you do leave assertions in release, then you might as well implement your assertion mechanism in terms of exceptions.

Share this post


Link to post
Share on other sites
Promit    13246
Sphet -- that's a terrible use of assert, that should be a thrown exception that is handled higher up in the code.

SiCrane -- I don't think a release time assert should fail gracefully either. Just like a debug time assert, you die on the spot. The idea behind an assertion is that if it fails, you are already completely screwed. DaBono's suggestion of letting the user save their game is even worse, since if save games are affected by the mistake and are now in an inconsistent state, you've permanently fried the user's save game (or left it in a state that could damage future execution of your program). If in the situation that a crash is only a matter of time (be it milliseconds or minutes), you may as well assert and stop now.

Share this post


Link to post
Share on other sites
SiCrane    11839
So you want to leave an open file handle that potentially prevents the program from being uninstalled just because you didn't feel like cleaning up from a crash?

Share this post


Link to post
Share on other sites
Promit    13246
Quote:
Original post by SiCrane
So you want to leave an open file handle that potentially prevents the program from being uninstalled just because you didn't feel like cleaning up from a crash?


Assertions are major problems, things that you believed were never possible. It's a reasonable assumption that if the assertion failed, you're about to crash. Your file handles are going to be left dangling anyway. And most OSes nowadays are able to release the file handles when a process dies, if it comes to that. It's between a crash and an assert->crash. May as well take the latter.

Share this post


Link to post
Share on other sites
SiCrane    11839
Thinking you're screwed is no reason not to make an attempt to clean up after yourself. And misbehaved programs on XP even with Service Pack 2 can still leave file handles in such a state that XP will refuse to delete the files even after reboots. Maybe Vista will be magical and wonderful and safe from programmer errors, but I somehow doubt it.

Share this post


Link to post
Share on other sites
SirLuthor    364
Quote:
Original post by Promit
Quote:
Original post by Nitage
There's no point in releasing an exectuable containing code to catch errors you've already fixed (any bugs in your program won't have corresponding assertion failures, else you'd know about the bug and have fixed it).


Are you ready to make the claim that your release code can never fail an assert condition, no matter what corner cases are reached?

That's the whole point of asserts, they are things that will never happen, unless the programmer makes a mistake somewhere in his code, or your program goes into an unpredictable death spiral for some odd, often hardware-related, reason. And generally the programmer's mistakes are ironed out before the final release. You have them in debug builds so that when that happens because the programmer did something, you can fix it. It's not an error handling mechanism, it's a way for a programmer to make absolute assertions about things that should always be correct. Ugh. I fear I made a hash of that explanation. Assertions are an obviously hard thing to describe. But just look at the word, or look it up if you don't know what it signifies.

You don't use assert as a debugging mechanism, if the programmer did his job, asserts wouldn't even be necessary, but be cause they're human, just like everyone else, they make mistakes, and thus, we have asserts to deal with that. It's like the standard libraries exceptions. You have runtime_error, and you have logic_error. The equivalent of runtime_error is exceptions or other means of error handling, and the equivalent of logic_error is asserts.

[Edited by - SirLuthor on October 7, 2005 3:04:33 PM]

Share this post


Link to post
Share on other sites
Promit    13246
Quote:
Original post by SirLuthor
That's the whole point of asserts, they are things that will never happen, unless the programmer makes a mistake somewhere in his code, or your program goes into an unpredictable death spiral for some odd, often hardware-related, reason.


The point of asserts is that they should never happen. But guess what, they do. The cases may be few and far between, but impossible conditions will go ahead and happen anyway. Limping along in an inconsistent state that could have been detected and killed is not a good idea.

Share this post


Link to post
Share on other sites
Promit    13246
SiCrane -- If an assert triggers, it means something even worse than "an exceptional event" has happened, something so serious that you didn't think it was possible. Program execution from that moment onwards is at best questionable, and at worst completely undefined. Normal clean up procedures might even involve more failure and inconsistent state, and in some cases that inconsistent state may get written to disk and damage future execution. Yes, resource handles are a bit of a concern, but if asserts are being used correctly and one fires, it's not necessarily even safe to try to let go of resources normally.

Share this post


Link to post
Share on other sites
SiCrane    11839
By that logic you shouldn't try displaying what assertion was tripped since your program is in such an undefined state that even doing output would be dangerous. So why bother keeping the assertions in your code?

Now the nice thing about protected mode OS's is that the kernel itself is insulated from bad things that happen in your code. And modern OS's make the assumption that they will get bad input (for whatever reason) from the applications every now and then. This means that they are check for things like closing handles that aren't actually open. So why not try closing those handles?

Share this post


Link to post
Share on other sites
Promit    13246
Well, if you're keeping a definitive list of handles somewhere that you can access and close without any side effects at all, then go right ahead.

But suppose the failure is within that definitive list? Better yet, what if the assertion was triggered while trying to remove a handle from the list? Now when you try to flush that list, chances are you'll assert again and attempt to flush the list again and assert again...et cetera. Hello re-entrance and infinite recursive loop.

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