0

Here's an .ashx handler that creates a series of Task objects which are then executed in a foreach loop.

        List<Task<Action>> list = new List<Task<Action>>();

        list.Add(Task.Factory.StartNew<Action>(() => AddHeader(graphics, request, reportParameters)));
        list.Add(Task.Factory.StartNew<Action>(() => AddSubmitter (graphics, request, reportParameters)));
        list.Add(Task.Factory.StartNew<Action>(() => AddActivity(graphics, request, reportParameters)));
        list.Add(Task.Factory.StartNew<Action>(() => AddCharts(graphics, request, reportParameters)));

        //Task.WaitAll(list.ToArray()); //AY - This seems unnecessary
        foreach (var action in list.Select(t => t.Result))
        {
            action();
        }

Each of the methods that return an Action does a CPU intensive work like the first method below.

private Action AddHeader(XGraphics graphics, HttpRequest request, ReportParameters reportParameters)
    {
        DataSet ds = new myData().ClientHeader(reportParameters);

        // This will be a function waiting to be called.
        return () =>
        {
          // Some CPU intensive operation code here.
        };
    }

Can someone please help me understand if this code is useful at all 1) from an asynchronous or multiple task perspective? 2) Is this code even creating multiple threads from the ASP.NET threadpool?

When I debug the app, it shows 4 separate tasks on my machine just before it runs the foreach loop. Each of those tasks retrieve the dataset and I think that's because that part of the code in each method runs before the return () Action statement.

So is the whole purpose of this code to enable multiple database calls in separate tasks before executing each CPU intensive action method? Or is the code really doing asynchronous stuff?

Ashish Yengkhom
  • 31
  • 8
  • There is a benefit on using task async patterns in asp.net - the workload is apparently moved onto Threads outside the app pool meaning you can get more concurrency and more sessions. Not sure how that relates to your implementation specifically hence only a comment. – kidshaw Mar 31 '15 at 18:43
  • 2
    @kidshaw No, the primary benefit of using asynchronous patterns in ASP.NET is allowing you to *use less threads*, not to use different threads. This allows the thread pool to spend more time doing productive work, handling requests, rather than non-productive work, like sitting around waiting for IO. – Servy Mar 31 '15 at 18:46

1 Answers1

1

It is not the best way to utilize tasks that represent thread pool execution units:

  1. Because each such task will block one thread from the thread pool, and ASP.NET uses the same thread pool as the tasks, you will reduce the amount of concurrent requests by a factor of five.
  2. Utilizing thread pool threads for database(I/O) bound code is ineffective, because thread pool threads will just sit idle, so use async code for database calls.
  3. But it is not the last problem - foreach (var action in list.Select(t => t.Result)) will block the execution till it gets the Result from each task sequentially. It means that returned actions will not be executed as soon as they are ready to execute, they will be executed in the same order as they were added to the list negating any potential advantage that could have been gained had we executed them as soon as they are ready.

So, as a conclusion:

Such code just starts 4 database calls in the least resource-effective way, and then squanders 4 misused thread pool threads executing their "continuation" on the original thread in an order that doesn't utilize the possibility that 4 task may execute in arbitrary order.

P.S.: And even when you really need to do CPU-intensive processing do not use Factory.StartNew - use Task.Run. It is not the actual error, because they basically do the same thing, but rather a good recommendation.

Community
  • 1
  • 1
Eugene Podskal
  • 10,270
  • 5
  • 31
  • 53
  • if Factory.StartNew is replaced by Task.Run, wouldn't it still consume a thread from the ASP.NET threadpool? If so, are you recommending using await async for my CPU intensive operations in ASP.NET? – Ashish Yengkhom Mar 31 '15 at 19:41
  • Task.Run is just a bit simplified version of Factory.StartNew, so it will still use the same thread from the same thread pool. Async-await is the way to work with I/O in ASP.NET (and everywhere else too). Compute-bound code shall still be executed on some thread, so whether you will increase user waiting time (using single thread) or reduce scalability (with more Task threads) is your own choice. – Eugene Podskal Mar 31 '15 at 20:17
  • @AshishYengkhom **Both** `StartNew` and `Run` will, if passed the default task scheduler, run the delegate in a thread pool thread. Both can of course be passed a task scheduler that runs the code without using a thread pool thread. – Servy Mar 31 '15 at 20:17