Semaphore blocks though it isn't full

Question

I am currently trying to optimize an old and very poorly written class which processes a lot of data and thus can easily take multiple hours to run through a set of data. Collecting the data already takes a lot of time and this is what I try to improve here. I know this is rather smelly code, but it's just a test if this even improves anything, so please just focus on the issue:

I tried SemaphoreSlim and Semaphore in order to reduce the amount of tasks running concurrently. My data set would generate about 70 tasks which would probably result in thread starvation and overall performance degradation. At least it turned out to become a lot less responsive. So I tried to keep it at 5 tasks at the same time for better overall throughput.

Now when I try to wait for my task to enter the sempahore it blocks (slim semphore using await also blocks) but it never enters even though the semaphore isn't full. This code is inside an async method as slight contextual hint.

Semaphore throttle = new Semaphore(0, 5);

try
{
    foreach (var folder in folders)
    {
        // Wait in case there are already 5 tasks running to reduce thread starvation
        collectionTasks.Add(Task.Run( () =>
        {
            // ReSharper disable once AccessToDisposedClosure
            throttle.WaitOne();
            return GetGapProfiles(folder.Value, progress, token);
        }, token).ContinueWith(
            t =>
            {
                // ReSharper disable once AccessToDisposedClosure
                throttle.Release();
                return t.Result;
            }, TaskContinuationOptions.None));
    }

    // When all are loaded concat all results into one collection
    await Task.WhenAll(collectionTasks);
}
catch (Exception ex)
{
    Log.Error(ex, "Failed to collect profiles.");
}
finally
{
    throttle.Dispose();
}

I just don't understand why this blocks and never ever enters GetGapProfiles. Can anyone explain this?

Answer 1

public static class perTaskThrottle
{
    /// <summary>
    /// Run multiple tasks in parallel - up to concurrentTasks tasks may run at any one time
    /// </summary>
    /// <typeparam name="TInput"></typeparam>
    /// <typeparam name="TResult"></typeparam>
    /// <param name="sourceItems"></param>
    /// <param name="func"></param>
    /// <param name="concurrentTasks"></param>
    /// <returns></returns>
    public static Task<IDictionary<TInput, TResult>> ForEachAsyncThrottled<TInput, TResult>(
        this IEnumerable<TInput> sourceItems,
        Func<TInput, Task<TResult>> func,
        int concurrentTasks = 1)
    {
        return ForEachAsyncThrottled(sourceItems, func, CancellationToken.None, concurrentTasks);
    }

    /// <summary>
    /// Run multiple tasks in parallel - up to concurrentTasks tasks may run at any one time
    /// </summary>
    /// <typeparam name="TInput"></typeparam>
    /// <typeparam name="TResult"></typeparam>
    /// <param name="sourceItems"></param>
    /// <param name="func"></param>
    /// <param name="token"></param>
    /// <param name="concurrentTasks"></param>
    /// <returns></returns>
    public static async Task<IDictionary<TInput, TResult>> ForEachAsyncThrottled<TInput, TResult>(
        this IEnumerable<TInput> sourceItems,
        Func<TInput, Task<TResult>> func,
        CancellationToken token,
        int concurrentTasks = 1)
    {
        var result = new ConcurrentDictionary<TInput, TResult>();

        var tasksList = new List<Task>();
        using (var semaphoreSlim = new SemaphoreSlim(concurrentTasks))
        {
            foreach (var item in sourceItems)
            {
                token.ThrowIfCancellationRequested();

                // if there are already concurrentTasks tasks executing, pause until one has completed ( semaphoreSlim.Release() )
                await semaphoreSlim.WaitAsync(perTimeSpanHelper.Forever, token).ConfigureAwait(false);

                token.ThrowIfCancellationRequested();

                Action<Task<TResult>> okContinuation = async task =>
                {
                    // the task has already completed if status is CompletedOk, but using await once more is safer than using task.Result
                    var taskResult = await task;
                    result[item] = taskResult;
                };

                // ReSharper disable once AccessToDisposedClosure
                Action<Task> allContinuation = task => semaphoreSlim.Release();

                tasksList.Add(func.Invoke(item)
                    .ContinueWith(okContinuation, TaskContinuationOptions.OnlyOnRanToCompletion)
                    .ContinueWith(allContinuation, token));

                token.ThrowIfCancellationRequested();
            }

            if (!token.IsCancellationRequested)
            {
                await Task.WhenAll(tasksList).ConfigureAwait(false);
            }
        }

        return result;
    }
}

So in your case you could use

var results = folders.ForEachAsyncThrottled( (f) => GetGapProfiles(f.Value), token, 5);