Thread-safe one time calculation best practices

Question

It's quite common that I need a property in my class which needs to be calculated and cached.

Generally I use a lock and a boolean top check if it's processed or not. Sometimes I do it in accessors.

What's the performance hit of this approach? Is there any better way to it.

Sample Code of my common approach to this:

   Sub Main()
     Dim X AS New X()

     For i AS Integer = 0 To 50
      Dim Thr AS New Threading.Thread(ADdressOF X.ProcessData )
      Thr.Start()
     Next

    End Sub

Private Class X

    Private DataCached AS Boolean 
    Private ProcessedData AS String 
    Private Lock AS New Object()
    Public Function ProcessData() AS String

    Synclock Lock
     IF NOT DataCached Then
      DataCached = True
      ProcessedData = DoStuff()
     End If
    End Synclock

     Console.Writeline(ProcessedData)  
     Return ProcessedData
    End Function


    Function DoStuff() AS String 
     Threading.Thread.Sleep(1000)
     Console.Writeline("Processed")
     return "stuff"
    End Function

End Class

EDIT :

This is something that need to be calculated when accessed because it keeps changing. Constructor calculation doesn't help in here. (sample is a really simplified version of what I'm doing)

Answer 1

This is the only way, there could be some other system library to do this, but eventually that library also would do same thing internally.

Answer 2

You can improve the concurrency with a double-check optimization:

If Not DataCached Then
    Synclock Lock
    If Not DataCached Then
        ProcessedData = DoStuff()
        DataCached = True ' Set this AFTER processing
    End If
End Synclock

This will avoid the critical section after the first init.

Answer 3

Is it critical that it is never calculated twice? i.e. if two threads happened to ask for it at the same time, and calculate the value independently, is that a show-stopper? In most cases, it isn't - in which case, just check for null (since it is a string): (example in C#, apologies):

   if(processedData == null) {
       processedData = DoStuff();
   }
   return processedData;

All subsequent calls should see the new value (I don't think we'll need volatile if it is hidden inside a property/method).

This has the advantage of being lock-free and simple.

Another trick is to use a static property of a nested class:

string SomeValue {
   get {return MyCache.SomeValue;}
}
static class MyCache {
    public static readonly string SomeValue;
    static MyCache() {
         SomeValue = DoStuff();
    }
}

This is calculated lazily, but the rules of static initializers mean that it is guaranteed to run once only (excluding reflection).

Answer 4

Firslty, I would move the caching outside of your class that contains the business logic and keep your business logic pure and allow you to control the caching independent of the application. But that's not your question ...

You need did not mention if you'll take more of a hit potentially calculating the things multiple times, until the cache is hot. The simple approach there would be:

if (Cache["key"] == null)
  Cache["key"] = obj.processData();

return Cache["key"];

Cache itself should ensure that this is safe.

If you wish to explicitly block whilst the cache is being populated, then you already have the semantics to do so in your code above, however I recommend this change:

if (Cache["key"] == null) {
  Synclock blockIfProcessing
    if (Cache["key"] == null) 
       Cache["key"] = obj.processData();
  End Synclock
}

return Cache["key"];

Basically this stops you blocking on every call once the cache is hot and will yield better performance as well as protecting you more from potential race conditions.

Bear in mind, as soon as you have two different locks, you open yourself up to potential deadlocks (and that's far beyond the scope of this thread).

Look for a .Net caching solution.