The dangers of static variables

Started by
15 comments, last by David Neubelt 16 years, 9 months ago
This post is meant to warn of the dangers of using of a static variable in commonly included header file. This bug was discovered at our company and I know that this trick is commonly used but the implications aren't widely known. This also is not specific to one linker, gcc, sn, and visual studio all suffered from this problem. Some of you, who have written a custom assertion functions, may be familiar with an old trick to skip assertions. The idea is when an assertion is hit the user may choose to skip all assertions on that line. For example, there may be a benign assertion that gets called every frame of your game. Instead of having to call over a programmer to fix the assert and push a new build the artist or designer can simply ignore all assertions from that spot in the code and continue working. One of the ways to accomplish this is have a static variable name created from the file and line number like this #define Assert_(e) static bool skipassert##__LINE__##__FILE__ = false; / if(skipassert##__LINE__##__FILE__ == false) / // -- do assertion logic When the prompt for the assert is displayed and the user chooses to ignore all asserts on that line we can simply toggle the bool and the assert will stop being triggered. There is a very big problem with this code that can be detrimental to a large code base. Using static variable names in a header file that is included in many different compliation units will cause the linker to slow down to a crawl. Removing the static variable from the assert caused our link times to go from 20-30 minutes down to 20 seconds. There are a couple possible explanations of this but I do not know of the real reason. The first reason could be because the variable name is very long. Resolving the name across all libraries will take much longer then a shorter name. This isn't very likely, but still possible because long link times only occur when symbols are generated. The variable is static and static variables should take object space and not use their symbol so it's probably not the long name. The reason that was explained to me that may be more likely is when a variable is static in a header file it will be declared once for every time the header is compiled. If the functions are emitted as select-any COMDATs there is a cost to recognize that when multiple instances of the function are seen and discarded where the linker has to go back and find the associated data and discard it too. Happy programming, -= Dave
Graphics Programmer - Ready At Dawn Studios
Advertisement
Would the problem you're describing be as important if you had only used this assertation trick in .cpp files?
Quote:Original post by Trillian
Would the problem you're describing be as important if you had only used this assertation trick in .cpp files?


This is only a problem with static variables in header files. Our new implementation that doesn't cause 20 minute link times looks like this.

    // -- base assert definition    #define Assert_(mask, e, msg) do {                                                                  if(!(e)) {                                                                                          uint32 returnaddr;                                                                       GetRA_(returnaddr);                                                                                  NRadEngine::CJournal::Log msg;                                                                  if(CJournal::ShowAssert(mask, #e, __FILE__, __LINE__, __FUNCTION__,                                                     returnaddr))                                                         { Break_ }                                                                              } } while(0)

I posted this because I've seen a few publications and code bases that use static variables in a header file. It's fairly common.

EDIT: source is missing \ on each line because it messes up formatting in source tags
Graphics Programmer - Ready At Dawn Studios
Quote:Original post by David Neubelt
This is only a problem with static variables in header files.

These static variables aren't in header files. The fact that the definition of the macro that defines the static variables appears in a header file doesn't cause the static variables to exist in the header file.

If static variables are the cause of the problem, they would cause it just as well if the macro definition instead appeared directly in each source file. The problem isn't "static variables in header files" but "static variables".

A further nitpick is that "skipassert##__LINE__" won't do what you imply it does, and whilst there are workarounds to make it do what is intended, there is no way that the value of __FILE__ could ever be part of a valid variable name, since __FILE__ is replaced with a string.
Quote:Original post by Nathan Baum
These static variables aren't in header files. The fact that the definition of the macro that defines the static variables appears in a header file doesn't cause the static variables to exist in the header file.

If static variables are the cause of the problem, they would cause it just as well if the macro definition instead appeared directly in each source file. The problem isn't "static variables in header files" but "static variables".


According to a developer, on Microsoft's linker, I was corresponding with during this ordeal, he told me that the cost would not apply if the assert were not in a header file.

Quote:A further nitpick is that "skipassert##__LINE__" won't do what you imply it does, and whilst there are workarounds to make it do what is intended, there is no way that the value of __FILE__ could ever be part of a valid variable name, since __FILE__ is replaced with a string.


We have multiple shipping products on multiple platforms and compilers with that code. I'm pretty sure it works. (EDIT: it doesn't quite work and the code doesn't look like that)

[Edited by - David Neubelt on July 14, 2007 10:10:46 PM]
Graphics Programmer - Ready At Dawn Studios
Quote:Original post by David Neubelt
According to a developer, on Microsoft's linker, I was corresponding with during this ordeal, he told me that the cost would not apply if the assert were not in a header file.


I can get further clarification on why it only applies for header files if you are curious why.

-= Dave
Graphics Programmer - Ready At Dawn Studios
The only way I can see this having any effect on link times is if you were doing the assert in inline functions that were getting used very frequently, in which case the linker would have to resolve those symbols to the same memory location. But as Nathan pointed out, what you have shown wouldn't even compile since __FILE__ is replaced with a string. A lie, it does compile because your variable is literally called "skipassert__LINE____FILE__" because the macros aren't expanded before concatenating the symbol. Ever used this macro twice within the same scope?

Quote:Original post by David Neubelt
According to a developer, on Microsoft's linker, I was corresponding with during this ordeal, he told me that the cost would not apply if the assert were not in a header file.


The linker doesn't even know header files exist, that's what the preprocessor is for.
"Voilà! In view, a humble vaudevillian veteran, cast vicariously as both victim and villain by the vicissitudes of Fate. This visage, no mere veneer of vanity, is a vestige of the vox populi, now vacant, vanished. However, this valorous visitation of a bygone vexation stands vivified, and has vowed to vanquish these venal and virulent vermin vanguarding vice and vouchsafing the violently vicious and voracious violation of volition. The only verdict is vengeance; a vendetta held as a votive, not in vain, for the value and veracity of such shall one day vindicate the vigilant and the virtuous. Verily, this vichyssoise of verbiage veers most verbose, so let me simply add that it's my very good honor to meet you and you may call me V.".....V
Quote:Original post by joanusdmentia
The only way I can see this having any effect on link times is if you were doing the assert in inline functions that were getting used very frequently, in which case the linker would have to resolve those symbols to the same memory location.
Yes you are correct - I didn't relay all of the information. The cost only applies when the assert is in a header file and the function containing the assert has multiple instances of the assert.

Quote:But as Nathan pointed out, what you have shown wouldn't even compile since __FILE__ is replaced with a string.


Like I said, multiple shipping products, I'm not making this up but since you refuse to believe me then please compile this.
#define MyAssert static bool eAssertSkipped_##__FILE__##__LINE__ = false;int main() {    MyAssert;    return 0;}


-= Dave

[/source]
Graphics Programmer - Ready At Dawn Studios
I corrected my post as you were writing your reply. It compiles, but try using it twice and it won't.

#define MyAssert static bool eAssertSkipped_##__FILE__##__LINE__ = false;int main() {    MyAssert;    MyAssert; // ERROR: eAssertSkipped___FILE____LINE__ already defined    return 0;}

"Voilà! In view, a humble vaudevillian veteran, cast vicariously as both victim and villain by the vicissitudes of Fate. This visage, no mere veneer of vanity, is a vestige of the vox populi, now vacant, vanished. However, this valorous visitation of a bygone vexation stands vivified, and has vowed to vanquish these venal and virulent vermin vanguarding vice and vouchsafing the violently vicious and voracious violation of volition. The only verdict is vengeance; a vendetta held as a votive, not in vain, for the value and veracity of such shall one day vindicate the vigilant and the virtuous. Verily, this vichyssoise of verbiage veers most verbose, so let me simply add that it's my very good honor to meet you and you may call me V.".....V
Quote:Original post by joanusdmentia
I corrected my post as you were writing your reply. It compiles, but try using it twice and it won't.


Hmm - you're right - I wonder how we got it to work, I need to dig up source history when I get to work and correct the original post.

Graphics Programmer - Ready At Dawn Studios

This topic is closed to new replies.

Advertisement