Question

Here is the scenario. I've got a class that will be accessed by multiple threads (ASP.NET) that can benefit from storing a result in a write-once, read-many cache. This cached object is the result of an operation that cannot be performed as part of a static initializer, but must wait for the first execution. So I implement a simple null check as seen below. I'm aware that if two threads hit this check at the same moment I will have ExpensiveCalculation calculated twice, but that isn't the end of the world. My question is, do I need to worry about the static _cachedResult still being seen as null by other threads due to optimizations or other thread caching. Once written, the object is only ever read so I don't think full-scale locking is needed.

public class Bippi
{

   private static ExpensiveCalculation _cachedResult;

   public int DoSomething(Something arg)
   {

      // calculate only once.  recalculating is not harmful, just wastes time.
      if (_cachedResult == null);
         _cachedResult = new ExpensiveCalculation(arg);


      // additional work with both arg and the results of the precalculated
      //    values of _cachedResult.A, _cachedResult.B, and _cachedResult.C
      int someResult = _cachedResult.A + _cachedResult.B + _cachedResult.C + arg.ChangableProp;
      return someResult;

   }

}

public class ExpensiveCalculation
{

   public int A { get; private set; }
   public int B { get; private set; }
   public int C { get; private set; }

   public ExpensiveCalculation(Something arg)
   {
      // arg is used to calculate A, B, and C
   }

}

Additional notes, this is in a .NET 4.0 application.

Was it helpful?

Solution

My question is, do I need to worry about the static _cachedResult still being seen as null by other threads due to optimizations or other thread caching.

Yes, you do. That's one of the primary reasons volatile exists.

And it's worth mentioning that uncontested locks add an entirely negligible performance cost, so there's really no reason to just to just lock the null check and resource generation, as it's almost certainly not going to cause any performance problems, and makes the program much easier to reason about.

And the best solution is to avoid the question entirely and use a higher level of abstraction that is specifically designed to solve the exact problem that you have. In this case, that means Lazy. You can create a Lazy object that defines how to create your expensive resource, access it wherever you need the object, and the Lazy implementation becomes responsible for ensuring that the resource is created no more than once, and that it is properly exposed to the code asking for said resource, and that it is handled efficiently.

OTHER TIPS

You need not need volatile, you - especially - need a memory barrier so that the processor caches synchronize.

I think you can altogether optimistically avoid locking, and yet avoid volatile performance penalties. You can test for nullability in a two-step fashion.

  object readonly _cachedResultLock = new object();

  ...

  if (_cachedResult == null)
  {
     lock(_cachedResultLock)
     {
         if (_cachedResult == null)
         {
             _cachedResult = new ExpensiveCalculation(arg);
         }
     }
   }

Here most of the time you will not reach lock and will not serialize access. You may serialize access only on first access - but will guarantee that work is not wasted (though may cause another thread to wait a bit while first one finishes ExpensiveCalculation).

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top