3

I have a function that needs to process items 3 at a time, and if the total time taken is less than x seconds, the thread should sleep for the remaining seconds before proceeding further.

So I'm doing the following:

    private void ProcessItems()
    {
        for (int i = 0, n = items.Count; i < n; i++)
        {
            Stopwatch stopwatch = new Stopwatch();
            stopwatch.Start();

            batch.Add(items[i]);

            if (batch.Count == 3 || i >= items.Count - 3)
            {
                List<Task> tasks = new List<Task>(3);

                foreach (Item item in batch)
                    tasks.Add(Task.Factory.StartNew(() => ProcessItem(item)));

                Task.WaitAll(tasks.ToArray());

                batch.Clear();
            }

            stopwatch.Stop();

            int elapsed = (int)stopwatch.ElapsedMilliseconds;
            int delay = (3000) - elapsed;

            if (delay > 0)
                Thread.Sleep(delay);
        }
    }

The ProcessItem function makes a webrequest and processes the response (callback). This is the function that takes a small amount of time.

However, if I understand tasks correctly, a thread can have multiple tasks. Therefore, if I sleep the thread, other tasks can be affected.

Is there a more efficient way to achieve the above, and can tasks be used within Parallel.Foreach?

MichaC
  • 13,104
  • 2
  • 44
  • 56
Ivan-Mark Debono
  • 15,500
  • 29
  • 132
  • 263

3 Answers3

2

Tasks run on automatically managed threads. There is nothing intrinsically wrong with blocking a thread. It is just a little wasteful.

Here is how I would implement this very cleanly:

MyItem[] items = ...;
foreach(MyItem[] itemsChunk in items.AsChunked(3)) {
 Parallel.ForEach(itemsChunk, item => Process(item));
 //here you can insert a delay
}

This wastes not a single thread and is trivially simple. Parallel.ForEach used the current thread to process work items as well, so it does not sit idle. You can add your delay logic as well. Implementing AsChunked is left as an exercise for the reader... This function is supposed to split a list into chunks of the given size (3). The good thing about such a helper function is that it untangles the batching logic from the important parts.

Community
  • 1
  • 1
usr
  • 168,620
  • 35
  • 240
  • 369
  • 1
    Yes. If you do not want to burn a thread for that, use `await Task.Delay`. That forces the entire async model on you, though. You must change all callers to use Task as well. If you have few threads it does not matter from a perf standpoint which model you chose because a thread has constant overhead. It does not make the stuff that runs on it slower or faster compared to async. I recommend against using async if you don't need it because it complicates stuff. – usr Oct 13 '13 at 13:26
  • How does await Task.Delay fit in your example then? – Ivan-Mark Debono Oct 13 '13 at 13:40
  • You insert `await Task.Delay(3000)` in the marked spot, mark the method `async`, change the return type to `Task` and now have to change all callers to await that returned task. I think you should research a bit how async works, and then decide whether you need that or not. Probably: not. Are you concerned about a specific problem? – usr Oct 13 '13 at 13:45
  • So await Task.Delay(3000) returns an empty task that waits for 3000 then? – Ivan-Mark Debono Oct 13 '13 at 13:55
  • @Ivan-MarkDebono No, `Task.Delay()` returns a `Task` that waits for the given time, `await Task.Delay()` asynchronously waits for that time. – svick Oct 13 '13 at 14:24
  • What is AsChunked ? implementation ? – Kiquenet Jan 01 '14 at 12:47
1

Use

Task.Delay 

instead

    static async Task DoSomeProcess()
    {
        await Task.Delay(3000);
    }

You are right, Thread.Sleep would block other tasks

Yes you can pair async/await pattern with Parallel.

MichaC
  • 13,104
  • 2
  • 44
  • 56
0

Your ProcessItems method can be very easily transformed into async version ProcessItemsAsync (I didn't validate the "batching" logic):

private async Task ProcessItemsAsync()
{
    for (int i = 0, n = items.Count; i < n; i++)
    {
        Stopwatch stopwatch = new Stopwatch();
        stopwatch.Start();

        batch.Add(items[i]);

        if (batch.Count == 3 || i >= items.Count - 3)
        {
            List<Task> tasks = new List<Task>(3);

            foreach (Item item in batch)
                tasks.Add(Task.Run(() => ProcessItem(item)));

            await Task.WhenAll(tasks.ToArray());

            batch.Clear();
        }

        stopwatch.Stop();

        int elapsed = (int)stopwatch.ElapsedMilliseconds;
        int delay = (3000) - elapsed;

        if (delay > 0)
            await Task.Delay(delay);
    }
}

The only benefit would be that you don't block the ProcessItems thread with Task.WaitAll() and Thread.Sleep(), as @usr pointed out in his answer. Whether to take this approach or Parallel.ForEach one probably depends on the running environment of your code. Async/await won't make your code run faster, but it will improve its scalability for server-side execution, because it may take less threads to run, so more clients could be served.

Note also that now ProcessItemsAsync is itself an async task, so to keep the flow of the code which calls it unchanged, you'd need to call it like this:

ProcessItemsAsync().Wait();

Which is a blocking call on its own and may kill the advantage of async we just gained. Whether you can completely eliminate blocks like this in your app or not, largely depends on the rest of the app's workflow.

noseratio
  • 59,932
  • 34
  • 208
  • 486