Sunday, February 6, 2011

Lazy Logging Parameter Evaluation With Variadic Macros

This entry is not rocket science, and probably won't be that informative to experienced programmers, but I've seen commercial code bases get something as simple as this wrong. It requires compiler support for variadic macros, which have been in Visual C++ for a while and are also supported by later versions of GCC.  

Most games have some sort of logging system. Debugging by printf is one of the first debugging tools most programmers learn. While there are many other tools in the debugging toolbox, this particular one is usually not that far out of reach. Some problems just lend themselves to being solved by logging.

We want to minimize the performance impact of logging code, without having to limit the number of logging statements we place in code. We do not want to constantly recompile different configurations of the game with or without logging enabled. While compile time stripping of logging during development will have the least performance impact, there are many times when you may be at a tester, designer or artist's desk and need to log key information. Providing them with a custom build is a productivity hit for everyone involved. 

There are two main performance hits for logging:

1. The cost of the logging itself (writing to the debug window, to a file, to a console, etc)
2. The cost of parameter evaluation

Anyone who has put a log statement in a piece of code executed many times a frame knows it can absolutely kill performance, just by the fact that logging to a file or debug output can be time consuming itself. This first cost can be solved by a channel system that can selectively enable logging. Even beyond the performance cost, it is useful to enable different types of logging at different times. If you're debugging AI code, you are probably not interested in logging from the streaming system.  Log statements specify which channel they are on (say, by integer ID), and the logging code checks if that channel is enabled.

Where should this check occur? I've seen some code bases that do this in the logging function itself. This is a mistake, because even if you do not actually output anything, you are still paying the second cost, the cost of parameter evaluation.

Logging, by nature, is very string-intensive. Often you will output human-readable debug names for various assets and entities when logging. Strings computed as parameters to log statements often incur performance penalties - memory allocations, string operations, etc. In addition to string overhead, some information you may wish to log may not be particularly cheap to calculate.

float ThisFunctionIsVeryExpensiveToEvaluate()

LogPrintf(LOG_INFO, "Expensive but precious debug info: %g\n", ThisFunctionIsVeryExpensiveToEvaluate());

What we want is for the expensive function to only be evaluated if the LOG_INFO channel is enabled.

The way to do this is to put the channel check in the macro itself, and only call the log function if the check succeeds. Here's some sample code that accomplishes this using variadic macros:


// Define this to 0 to disable logging
#define ENABLE_LOGGING 1

const int LOG_ERROR=0x1;
const int LOG_WARNING=0x2;
const int LOG_INFO=0x4;

#if ENABLE_LOGGING
   // Simple channel system (you want IsLoggingChannelEnabled to be cheap,
   // but there are other ways to implement something like this)
   static int GlobalEnabledLogChannels;

   // Make sure your compiler inlines this function, as it will be called many
   // times
   // You may want to force it to be inlined using whatever compiler-specific
   // syntax is available to you.
   inline bool IsLoggingChannelEnabled(int channel)
   {
       return 0 != (GlobalEnabledLogChannels & channel);
   }

   // This overload is present to handle the case where the channel argument
   // is optional
   inline bool IsLoggingChannelEnabled(const char*)
   {
       return true;
   }

   // Note: I've seen many logging systems which make the log channel optional.
   // I'm going to handle this case to show how it is done, but if you always
   // require a log channel, this code becomes simpler (for instance, you can
   // make format a required argument to the macro, and not need the ## handling)
   void MyLogPrintf(const char* format, ...);
   void MyLogPrintf(int channel, const char * format,...);

   // The ## is some syntax magic to make GCC ignore the preceding
   // comma if no arguments after channel are present
   // This can happen if no channel is specified in the log print, as it is optional
   #define LogPrintf(channel, ...) \
      if(!IsLoggingChannelEnabled(channel)) {} else MyLogPrintf(channel, ##__VA_ARGS__)


#else

   #if _MSC_VER
      // __noop is Visual C++ specific syntax for "do nothing".
      #define LogPrintf(...) __noop
   #else
      // Compiler should strip this out - but always look at the disassembly to make sure!      inline void Noop()
     {
     }

      #define LogPrintf(...) Noop()
   #endif

#endif

// example log statements
void SomeFunction()
{
    LogPrintf("Hello world!\n");
    LogPrintf(LOG_ERROR, "You should see this very important error\n");
    LogPrintf(LOG_INFO, "Expensive info: %s\n",
    ThisFunctionIsVeryExpensiveToEvaluate().c_str());
}

Hopefully blogger didn't mangle the formatting of all that.

The key concept is to call IsLoggingChannelEnabled() in the macro itself. The if syntax it uses is specially constructed -- done this way it will not change the semantics of an if statement without braces. For example:

if (rand()%2 == 0)
    LogPrintf(LOG_INFO,"Rand was even\n");
else
    LogPrintf(LOG_INFO, "Rand was odd\n");

If we did something like this:

#define LogPrintf(channel, ...) \
if(IsLoggingChannelEnabled(channel)) MyLogPrintf(channel, ##__VA_ARGS__)

that would change the meaning of the above if statement, and the else case would be on if(IsLoggingChannelEnabled(channel)), not the original rand check!

A note on why I made the channel optional: In a particular legacy code base I was dealing with, the channel argument was optional on logging statements, and I had to handle that case without changing every log statement in the application. I wanted to show how you could support something like that.

The main drawback with this approach is an increase in executable size due to all the log channel checks being inlined and the performance hit of the check itself on each log statement. It really depends on your particular game whether you are willing to pay these costs in your development/in-house build or not.

5 comments:

  1. This comment has been removed by the author.

    ReplyDelete
  2. This comment has been removed by the author.

    ReplyDelete
  3. ((void)0) is, syntactically, a good replacement for __noop. I'm sure __noop pops an actual NOP in your code, which of course is not what you want. (To say nothing of its not working on gcc.)

    Regarding the check for enabledness of the log, you can put this inline in the macro. I'm actually a bit surprised this isn't done already - is there a reason?

    What I have is something vaguely like this one:

    #define LOG(L,...)\
    do {\
    extern int logging_enabled;\
    if(logging_enabled)\
    DoLog(&(L),__VA_ARGS__);\
    } while(0)

    (It should be easy to fill in the blanks.)

    The advantage of this is that you never have to worry about the IsLogEnabled function being inlined, so performance is (in theory) a bit better in your debug build.

    For the example I provide, where there's just one variable, there's no great advantage over extern'ing it in the header. If you generate the extern'd variable name from the log name, however, you can now configure things on a per-log basis without having to include some kind of log declaration macro in a header.

    (One minor difficulty here is making this work in a mixed C/C++ code base when using gcc, which seems to mangle variable names as well as function names. Easy solution, of course, but there's no automatic way to get the appropriate extern "C" into a global scope.

    (I don't have any great solution for this; personally, I just made the "declare log" macro I mentioned above, and if I'm going to use in C++ a log that was defined in a C file then I have to add the declaration macro at global scope with an extern "C" around it. The other way around... well, haven't needed that yet. And anyway, most of the time, I don't need to do this.)

    ReplyDelete
  4. It occurred to me after posting that maybe VC++ is hitting you with the /W4 "conditional expression is constant" warnings, and that's why you're avoiding do...while(0). You can fix this using the __pragma statement, which gives you the following monstrosity:

    #define END_MACRO\
    __pragma(warning(push)) __pragma(warning(disable:4127))\
    while(0)\
    __pragma(warning(pop))

    (__pragma works on VS2005; I assume it's supported on later versions too.)

    Your macros are all then, like:

    #define MACRO()\
    do {\
    /* stuff */\
    } END_MACRO

    Much better, to my eyes, than using "if", and often easier/more appropriate than trying to squeeze things into a single ternary expression.

    Of course, on other compilers END_MACRO would just expand to "while(0)".

    ReplyDelete
  5. what about simply using ; instead of this Noop function ?
    #define LogPrintf(...) ;

    ReplyDelete