The Risk of Micro-optimizations


A lot of things have been said over the years about premature optimization, and after running into the following bug I thought it served as a good example of a type of premature optimization that I call “micro-optimization”. That is, doing something quirky in order to save a tiny amount of RAM or CPU cycles. More often than not these quirks lead to bugs and less maintainable code, with no actual performance increase.


The following bug was in a class in an application designed to run on a modern desktop, and there would only be a few instances alive at any one time (say, 1-10 typically, maybe 100 in edge cases), so the amount of memory saved was negligible (if any).


This is a rather easy “spot the bug” exercise so I won’t give it away. 😉

DWORD WEATHER_SUNNY   = 0x0001;
DWORD WEATHER_RAINING = 0x0002;

struct CWeather
{
BOOL _fRaining:1;

CWeather(DWORD dwFlags)
{
_fRaining = (dwFlags & WEATHER_RAINING);
}

BOOL IsRaining()
{
return _fRaining;
}
};

int main()
{
CWeather weather(WEATHER_RAINING);

if (weather.IsRaining())
{
cout << “It’s raining!” << endl;
}
else
{
cout << “It’s not raining!” << endl;
}

return 0;
}


Note: Depending on your compiler, if you crank up your warning level high enough you should get a warning for this. (Although curiously I didn’t with VS 2005 and /W4).

Comments (5)

  1. Rick Schaut says:

    I cringe whenever I see a "(foo & flag)" expression where the result is used as a boolean, and I always code it up as "(foo & flag) != 0". The extra instruction is well worth the increase in maintainability.

    Rick

  2. Agreed.

    I’m usually pretty picky about being as clear/explicit/intentional as possible when writing code, such as using "if (p != NULL)" instead of "if (p)", but the "(foo & flag)" idiom is so common that it’s almost awkward to add the "!= 0" part.

  3. amit joshi says:

    BOOL IsRaining()

    {

    return _fRaining;

    }

    Returning uninitialized bits?

  4. Mike Dimmick says:

    There’s absolutely no point doing this since the compiler will not combine the single bits from multiple instances of the class. It will round up each instance to one whole byte. I don’t think it will add any extra padding between instances, so an array of 10 CWeather objects will take 10 bytes (plus overhead), whereas they’d take 40 bytes if you used a BOOL. But unless that pushes you over a page boundary somewhere, it will have very little effect on your program.

    I don’t have any quantitative analysis on how long 32-bit vs 8-bit operations on modern processors take, but the processor reads from main memory into cache in (IIRC) 64 byte chunks, at least for cacheable reads. Again, little benefit.