1

I'm using Asp.Net 4.0.

I also have a HashSet from which I want to create and run tasks asynchronously, waiting for them all to finish with a timeout applied. The code below is what I have so far, but given that there are so many ways to skin a cat with TPL, can this be improved on, or are there any critical issues?

 TaskWrapper[] taskWrappers = new TaskWrapper[nodeCount]; Task<ResponseWrapper>[] tasks = new Task<ResponseWrapper>[nodeCount]; int i = 0; foreach (var n in rt.Nodes) { tasks[i] = Task<ResponseWrapper>.Factory.StartNew(() => MyMethod.Submit(n.Data, timeout)); taskWrappers[i].LeadPointID = n.LeadPointID; // other wrapper stuff; taskWrappers[i].Task = tasks[i]; i++; } Task.WaitAll(tasks, timeout);

Having read the article kindly provided, it seems like this kind of code structure would be the preferred method of working:

class MyService2 { public int CalculateMandelbrot() { // Tons of work to do in here! for (int i = 0; i != 10000000; ++i) ; return 42; } } private async void MyButton_Click(object sender, EventArgs e) { await Task.Run(() => myService.CalculateMandelbrot()); }

However, as I've previously pointed out I don't have access to VS2012, so given that as a starting point, is there a VS2010 equivalent to this pattern?

Improve this question
5
  • If everything is working as required this feels like it should be on codereview.stackexchange.com. Commented Jul 22, 2014 at 9:59
  • With C# 4(which comes with .NET 4.0) you should be careful with capturing foreach loops variables. Commented Jul 22, 2014 at 10:03
  • Looks good. Nothing wrong with that. Especially nothing wrong with using synchronous blocking calls despite async being all the rage right now. I'd replace the imperative loop with a functional Select (e.g. rt.Nodes.Select(n => ...)). Commented Jul 22, 2014 at 11:39
  • @usr But blocking on a call means that thread becomes useless for the duration of the call. You're wasting resources. And this is easily avoidable. How come there's nothing wrong with it? Commented Jul 22, 2014 at 13:59
  • @dcastro all it does is save memory (mostly the managed stack). If you have the memory available the benefit of saving it is zero. Commented Jul 22, 2014 at 14:00

1 Answer 1

Reset to default
2

Task.WaitAll will block your main thread until all other tasks have completed. That's one less thread that could be being used to server more requests.

Instead, you should be using await Task.WhenAll. However, WhenAll does not have an overload that takes a timeout. A possible workaround is to combine it with a Task.Delay, and then wait for either all your tasks to finish or the delay task to finish:

await Task.WhenAny(Task.WhenAll(tasks), Task.Delay(timeout));

If you don't have access to VS 2012, then an alternative would be to attach a continuation task to WhenAny, and return that task. The framework will take care of the rest.

public Task<string> Post() { var tasks = //... var continuation = Task.WhenAny(Task.WhenAll(tasks), Task.Delay(timeout)) .ContinueWith(whenAnyTask => { //the rest of your code return "Hello"; }; return continuation; }

More importantly: do not use Task.Run (or StartNew) on an ASP.NET application!

You're taking threads from the threadpool to serve a client faster, but those threads could be being used to serve more clients! So you're telling other clients "hold on a minute, i'm focusing my resources on serving this guy first". You're also introducing a lot of needless overhead.

In a web app, it's good practice to use only one thread at a time to serve a client, and use asynchronous code for async I/O only.

Edit : Aron

Each thread in .net (x86) uses 1MB of RAM or 4MB on (x64). Even a fairly beefy server with 16GB of ram would ONLY be able to therefore run 4000 threads (assuming no memory usage anywhere else). That means that IF each request runs 4 threads, you can only concurrently handle 1000 requests.

Whereas a node.js server (which uses a similar model to ASP.Net async/await) is easily able to handle over 10000 concurrent requests on a single thread (read a much smaller memory footprint).

For a more detailed explanation, see Stephen Cleary's Task.Run Etiquette Examples: Don't Use Task.Run in the Implementation.

Improve this answer
Sign up to request clarification or add additional context in comments.

13 Comments

I believe 'await' isn't available to me in VS2010/Asp.Net 4.0
@dotnetnoob If you upgrade to vs2012, you could. The Async Targetting Pack enables async/await on .NET 4.0. Even if you don't want to upgrade, your code needs reworking. You should be doing everything synchronously, in one thread, and use tasks for async I/O only. See my updated answer.
It may be a little clearer if you know what I want to achieve. I need to make a number of Http POST requests to a third party service for each user. Some are syncrhonous and some like this are asynchronous where I need to wait for a response.
@dotnetnoob Even so, you should not be using StartNew. If those HTTP requests are async (as they should), the API will return a task. Your MyMethod.Submit method should return that task back to the controller, and then the controller awaits that task directly.
@dcastro I would however say instead of "You should be doing everything synchronously, in one thread, and use tasks for async I/O only." I would say that you should do everything single-thread asynchronously. The problem is that years of dogma have left us with an entire generation of developers that equates asynchronous with multi-threading. Again...see Stephen Cleary and Jon Skeet on the subject.
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.