Use a Task to avoid multiple calls to expensive operation and to cache its result

Question

I have an async method that fetches some data from a database. This operation is fairly expensive, and takes a long time to complete. As a result, I'd like to cache the method's return value. However, it's possible that the async method will be called multiple times before its initial execution has a chance to return and save its result to the cache, resulting in multiple calls to this expensive operation.

To avoid this, I'm currently reusing a Task, like so:

public class DataAccess
{
    private Task<MyData> _getDataTask;

    public async Task<MyData> GetDataAsync()
    {
        if (_getDataTask == null)
        {
            _getDataTask = Task.Run(() => synchronousDataAccessMethod());
        }

        return await _getDataTask;
    }
}

My thought is that the initial call to GetDataAsync will kick off the synchronousDataAccessMethod method in a Task, and any subsequent calls to this method before the Task has completed will simply await the already running Task, automatically avoiding calling synchronousDataAccessMethod more than once. Calls made to GetDataAsync after the private Task has completed will cause the Task to be awaited, which will immediately return the data from its initial execution.

This seems to be working, but I'm having some strange performance issues that I suspect may be tied to this approach. Specifically, awaiting _getDataTask after it has completed takes several seconds (and locks the UI thread), even though the synchronousDataAccessMethod call is not called.

Am I misusing async/await? Is there a hidden gotcha that I'm not seeing? Is there a better way to accomplish the desired behavior?

EDIT

Here's how I call this method:

var result = (await myDataAccessObject.GetDataAsync()).ToList();

Maybe it has something to do with the fact that the result is not immediately enumerated?

Answer 1

If you want to await it further up the call stack, I think you want this:

public class DataAccess
{
    private Task<MyData> _getDataTask;
    private readonly object lockObj = new Object();

    public async Task<MyData> GetDataAsync()
    {
        lock(lockObj)
        {
            if (_getDataTask == null)
            {
                _getDataTask = Task.Run(() => synchronousDataAccessMethod());
            }
        }
        return await _getDataTask;
    }
}

Your original code has the potential for this happening:

• Thread 1 sees that _getDataTask == null, and begins constructing the task
• Thread 2 sees that _getDataTask == null, and begins constructing the task
• Thread 1 finishes constructing the task, which starts, and Thread 1 waits on that task
• Thread 2 finishes constructing a task, which starts, and Thread 2 waits on that task

You end up with two instances of the task running.

Answer 2

Use the lock function to prevent multiple calls to the database query section. Lock will make it thread safe so that once it has been cached all the other calls will use it instead of running to the database for fulfillment.

lock(StaticObject) // Create a static object so there is only one value defined for this routine
{
    if(_getDataTask == null)
    {
         // Get data code here
    }
    return _getDataTask
}

Answer 3

Please rewrite your function as:

public Task<MyData> GetDataAsync()
{
    if (_getDataTask == null)
    {
        _getDataTask = Task.Run(() => synchronousDataAccessMethod());
    }

    return _getDataTask;
}

This should not change at all the things that can be done with this function - you can still await on the returned task!

Please tell me if that changes anything.

Answer 4

Bit late to answer this but there is an open source library called LazyCache that will do this for you in two lines of code and it was recently updated to handle caching Tasks for just this sort of situation. It is also available on nuget.

Example:

    Func<Task<List<MyData>>> cacheableAsyncFunc = () => myDataAccessObject.GetDataAsync();

    var cachedData = await cache.GetOrAddAsync("myDataAccessObject.GetData", cacheableAsyncFunc);

    return cachedData;

    // Or instead just do it all in one line if you prefer
    // return await cache.GetOrAddAsync("myDataAccessObject.GetData", myDataAccessObject.GetDataAsync);
}

It has built in locking by default so the cacheable method will only execute once per cache miss, and it uses a lamda so you can do "get or add" in one go. It defaults to 20 minutes sliding expiration but you can set whatever caching policy you like on it.

More info on caching tasks is in the api docs and you may find the sample app to demo caching tasks useful.

(Disclaimer: I am the author of LazyCache)