Cold Tasks and TaskExtensions.Unwrap

Question

I've got a caching class that uses cold (unstarted) tasks to avoid running the expensive thing multiple times.

public class AsyncConcurrentDictionary<TKey, TValue> : System.Collections.Concurrent.ConcurrentDictionary<TKey, Task<TValue>>
{
    internal Task<TValue> GetOrAddAsync(TKey key, Task<TValue> newTask)
    {
        var cachedTask = base.GetOrAdd(key, newTask);

        if (cachedTask == newTask && cachedTask.Status == TaskStatus.Created) // We won! our task is now the cached task, so run it 
            cachedTask.Start();

        return cachedTask;
    }
}

This works great right up until your task is actually implemented using C#5's await, ala

cache.GetOrAddAsync("key", new Task(async () => {
  var r = await AsyncOperation();
  return r.FastSynchronousTransform();
}));)`

Now it looks like TaskExtensions.Unwrap() does exactly what I need by turning Task<Task<T>> into a Task<T>, but it seems that wrapper it returns doesn't actually support Start() - it throws an exception.

TaskCompletionSource (my go to for slightly special Task needs) doesn't seem to have any facilities for this sort of thing either.

Is there an alternative to TaskExtensions.Unwrap() that supports "cold tasks"?

Juanjuana

why don't you rephrase your question? i have read it multiple times and still unable to understand what you want to achieve. perhaps same is the case with others that is why you haven't got any answer as yet – Orientation

What part don't you understand? Task.Unwrap returns a "Promise style" task which are already started, whereas I need Task instances to be unstarted so I can coalesce them in my cache. – Juanjuana

Answer 1

All you need to do is to keep the Task before unwrapping it around and start that:

public Task<TValue> GetOrAddAsync(TKey key, Func<Task<TValue>> taskFunc)
{
    Task<Task<TValue>> wrappedTask = new Task<Task<TValue>>(taskFunc);
    Task<TValue> unwrappedTask = wrappedTask.Unwrap();

    Task<TValue> cachedTask = base.GetOrAdd(key, unwrappedTask);

    if (cachedTask == unwrappedTask)
        wrappedTask.Start();

    return cachedTask;
}

Usage:

cache.GetOrAddAsync(
    "key", async () =>
    {
        var r = await AsyncOperation();
        return r.FastSynchronousTransform();
    });

Zaragoza

Answer 2

This answer is an attempted improvement on svick's answer. Before calling the GetOrAdd we can first call the TryGetValue, to avoid creating invariably cold tasks in case they are not needed:

public Task<TValue> GetOrAddAsync(TKey key, Func<Task<TValue>> valueFactory)
{
    ArgumentNullException.ThrowIfNull(valueFactory);
    Task<TValue> currentTask;
    if (base.TryGetValue(key, out currentTask))
        return currentTask;

    Task<Task<TValue>> newTaskTask = new Task<Task<TValue>>(valueFactory);
    Task<TValue> newTask = newTaskTask.Unwrap();

    currentTask = base.GetOrAdd(key, newTask);

    if (currentTask == newTask)
        newTaskTask.RunSynchronously(TaskScheduler.Default);

    return currentTask;
}

I've made also a couple more changes. I've used the RunSynchronously instead of the Start, because there is no reason to offload the invocation of the valueFactory to the ThreadPool. I've also passed the TaskScheduler.Default as argument, to comply with the CA2008 guideline.

This version is still not suitable for a production system IMO, since it suffers from a significant flaw: in case the valueFactory fails, the exception will be cached in the AsyncConcurrentDictionary<K,V>. In a caching system, you rarely, if ever, want to cache exceptions. In general you want to retry the failed operations again and again until they succeed. You can find a retrying version of the GetOrAddAsync here, that includes also a small memory-optimization (storing lightweight ValueTasks instead of Tasks).

Minister