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

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

Solution

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.