Caching expensive data in C++ - function-scoped statics vs mutable member variables

Question

I've got a relatively expensive data-fetching operation that I want to cache the results of. This operation is called from const methods, roughly like this:

double AdjustData(double d, int key) const {
  double factor = LongRunningOperationToFetchFactor(key);
  return factor * d;
}

I'd like AdjustData to remain const, but I want to cache out the factor so I only fetch it the first time. At present I'm using a mutable map<int, double> to store the result (the map being from key to factor), but I'm thinking using a function-scoped static might be a better solution - this factor is only needed by this function, and is irrelevant to the rest of the class.

Does that seem like a good way to go? Are there any better options? What things might I think about, particularly with regard to thread-safety.

Thanks,

Dom

Answer 1

I would wrap the implementation of LongRunningOperationToFetchFactor with something like this. I am using Boost scoped locks but you can so something similar with other locking frameworks.

#include <boost/thread/thread.hpp>
#include <boost/thread/mutex.hpp>
#include <map>

using namespace std;

static boost::mutex myMutex;
static map<int,double> results;

double CachedLongRunningOperationToFetchFactor( int key )
{

   {
       boost::mutex::scoped_lock lock(myMutex);

       map<int,double>::iterator iter = results.find(key);
       if ( iter != results.end() )
       {
          return (*iter).second;
       }
   }
   // not in the Cache calculate it
   result = LongRunningOperationToFetchFactor( key );
   {
       // we need to lock the map again
       boost::mutex::scoped_lock lock(myMutex);
       // it could be that another thread already calculated the result but
       // map assignment does not care.
       results[key] = result;
   }
   return result;
}

If this really is a long running operation then the cost of locking the Mutex should be minimal.

It was not quite clear from you question but if the function LongRunningOperationToFetchFactor is a member function of you class then you want the map the be mutable map in that same class. I single static mutex for access is still fast enough though.

Answer 2

I would not make this cache a local static. The mutable map is the solution for caching results. Otherwise it will make your function useless, as different objects of your class will share the same cache, as the local static cache is the same for all objects. You can use the local static if the result does not depend on the object though. But then i would ask myself why the function is a non-static member of your object, if it does not need to access any state of it.

As you say it should be thread-safe - if different threads can call the member function on the same object, you probably want to use a mutex. boost::thread is a good library to use.